Re: [PATCH v5 08/11] xen/arm: Enable the existing x86 virtual PCI support for ARM.

[Julien Grall <julien@xxxxxxx>]

On 11/10/2021 13:41, Bertrand Marquis wrote:
> Hi Jan,
Hi Bertrand,

> As Rahul is on leave, I will answer you and make the changes needed.
>
> > On 7 Oct 2021, at 14:43, Jan Beulich <jbeulich@xxxxxxxx> wrote:
> > Independent of this - is bare metal Arm enforcing this same
> > alignment restriction (unconditionally)? Iirc on x86 we felt we'd
> > better synthesize misaligned accesses.
> Unaligned IO access could be synthesise also on arm to but I would
> rather not make such a change now without testing it (and there is
> also a question of it making sense).
Yes it makes sense. I actually have an item in my TODO list to forbid 
unaligned access because they should not happen on any device we 
currently emulate.
Although, I am not aware of any issue other than the guest would shoot 
itself in the foot if this happens.
> So if it is ok with you I will keep that check and discuss it with Rahul
> when he is back. I will add a comment in the code to make that clear.
I am OK with it.

[...]

> > Throughout this series I haven't been able to spot where the HAS_VPCI
> > Kconfig symbol would get selected. Hence I cannot tell whether all of
> > this is Arm64-specific. Otherwise I wonder whether size 8 actually
> > can occur on Arm32.
> Dabt.size could be 3 even on ARM32 but we should not allow 64bit
> access on mmio regions on arm32.
Hmmm... Looking at the Armv7 and Armv8 spec, ldrd/strd (64-bit read) 
would not present a valid ISV. So I think it is not be possible to have 
dabt.size = 3 for 32-bit domain. But I agree we probably want to harden 
the code.
> So I will surround this code with ifdef CONFIG_ARM_64 and add a test
> for len > 4 to prevent this case on 32bit.
>
> To be completely right we should disable this also for 32bit guests but
> this change would be a bit more invasive.
I think the following should be sufficient:

if ( is_domain_32bit_domain() && len > 4 )
  return ...;

Cheers,

--
Julien Grall