Hi Julien,
> On 12 Oct 2021, at 16:04, Julien Grall <jul...@xen.org> wrote:
>
> 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 <jbeul...@suse.com> 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 ...;
With the last request from Roger to use the function implemented in
arch/x86/hw/io.c, the function will move to vpci.h so using is_32bit_domain
will not be possible without ifdefery CONFIG_ARM.
Also I have no access to the domain there.
So the best I can do for now would be something like:
#ifdef CONFIG_ARM_32
If (len == 8)
return false
#endif
A 32bit guest on 64bit xen would not be checked.
Would that be ok for now ?
I could add a comment in the code to warn about the limitation.
Cheers
Bertrand
>
> Cheers,
>
> --
> Julien Grall
