[PATCH] eal: fix alignment of RISCV xmm vector type

Bruce Richardson bruce.richardson at intel.com
Fri Nov 17 11:54:33 CET 2023


On Thu, Nov 16, 2023 at 08:45:35AM +0100, Morten Brørup wrote:
> > From: Stanisław Kardach [mailto:kda at semihalf.com]
> > Sent: Thursday, 16 November 2023 00.21
> > 
> > On Wed, Nov 15, 2023 at 10:31 PM Morten Brørup
> > <mb at smartsharesystems.com> wrote:
> > >
> > > > From: Tyler Retzlaff [mailto:roretzla at linux.microsoft.com]
> > > > Sent: Wednesday, 15 November 2023 22.17
> > > >
> > > > Fix the alignment for rte_xmm_t it should be 16 instead of 8 bytes.
> > > >
> > > > Fixes: f22e705ebf12 ("eal/riscv: support RISC-V architecture")
> > > > Cc: maz at semihalf.com
> > > > Cc: stable at dpdk.org
> > > > Signed-off-by: Tyler Retzlaff <roretzla at linux.microsoft.com>
> > > > ---
> > >
> > > Reviewed-by: Morten Brørup <mb at smartsharesystems.com>
> > >
> > > As mentioned in the other thread:
> > >
> > > We need to urgently decide if this bug should live on in DPDK 23.11,
> > or if the fix should be included although we are very late in the
> > release process.
> > >
> > > Stanislaw, what do you think?
> > Good catch! As for backporting I'm not sure of the urgency given that
> > our examples still use scalar instructions for handling xmm_t. The
> > question is whether there is a platform in use which has vector
> > extensions enabled and that utilizes DPDK. I'm not that sure of it
> > though I'd be happy to be proven wrong.
> 
> Can we extrapolate this, and also conclude that postponing this bugfix until the next ABI/API breaking release, DPDK 24.11, is not really going to hurt anyone?
> 
> Stanislaw, please confirm?
> 
> Bruce, I don't feel 100 % confident in making this postponement recommendation. Could you please provide a second opinion regarding the timing of fixing this bug? Or rather: do you have any strong arguments *against* postponing it to DPDK 24.11?
> 

Not sure I'm any better placed to make an argument either way! However, I
would very much tend to say that we should include this in 23.11, on the
basis that if it turns out to be important we can't backport it later
without affecting ABI. Right now, the code looks broken to me, and I'm also
struggling to find circumstances where increasing the alignment will
actually stop something from working. There could well be performance
implications of having extra padding, but things should still work, AFAIK.
On the other hand, if we don't include the fix, it is possible that a
system (possibly a future one) could break and segfault due to incorrect
vector code. I'd take a possible slowdown over a segfault!

Is my assessment correct, or perhaps I'm missing some detail.

/Bruce


More information about the stable mailing list