Re: [Xen-devel] [PATCH 4/4] xen/arm64: Emulate correctly the register {w, x}zr

[Julien Grall <julien.grall@xxxxxxxxxx>]

Hi Ian,

On 15/12/15 11:10, Ian Campbell wrote:
> On Fri, 2015-12-11 at 15:28 +0000, Julien Grall wrote:
>> On AArch64, the encoding 31 could be used to represent {x,w}sp or
>> {x,w}zr (See C1.2.4 in ARM DDI 0486A.d). The choice will depend how the
>> register field is interpreted by the instruction.
>>
>> All the instructions trapped by Xen (either via a sysreg access or data
>> abort) interprets the encoding 31 as zr. Therefore we don't have to
>> worry about decoding the instruction.
> 
> Is decoding the only way we could tell? i.e. there's no indication in the
> syndrome reg?

Unfortunately yes, the syndrome reg will only contain the encoding of
the transferred register.

However, by looking to the list of instruction that could be trapped by
Xen and contain valid ISS, the encoding 31 will always mean zr.

> Is there no way a data abort could result from an access which involved SP
> rather than XZ? e.g. if the OS was to (stupidly) point SP at an MMIO area
> and then try a stp x0, x1, [sp] for example? Ah, perhaps in that case we
> would get a trap with x0 or x1 but sp would be in the FAR and not in the
> HSR?

First if the instruction stp trap into Xen, the ISS would be invalid.

For AArch64, only loads and store of a single general-purpose register
would result to a valid ISS (see D7-1880 in ARM DDI 0487A.d).

If we take an instruction producing a valid ISS:

ldr x0, [x2]
str x0, [x2]

The ISS will always contains the transfered register, i.e x0 here. The
faulting address (x2) will be in FAR.

> 
> Or maybe a less lunatic case, could xenaccess not result in a faulting
> stack access? I suppose the answer there is that xenaccess faults are
> handled without actually caring which register it was and then the
> instruction is replayed in guest context?

xenaccess can't rely on the instruction syndrome because the fault may
happen with instruction producing invalid ISS.

If we ever need the register in xenaccess we have to decode by hand
every instruction.

> 
> Aside: if an stp traps, does the h/v get two exits, one for each register?
> I suppose it must.
> 
>> The register zr reprents the zero register, i.e it will always
> 
> "represents"
> 
>> return 0 and write to it is ignored. To handle properly this properties,
> 
> "these properties"
> 
>> 2 new helpers have been introduced {get,set}_user_reg to read/write a
>> value from/to a register. All the call to select_user_reg have been
> 
> calls
> 
>> replaced by these 2 helpers.
>>
>> Furthermore, the code to emulate encoding 31 in select_user_reg has been
>> dropped because it was invalid. For Aarch64 context, the encoding is
>> used for sp or zr. For AArch32 context, the ISS won't be valid for data
>> abort from AArch32 using r15 (i.e pc) as source/destination (See D7-1881
>> ARM DDI 0487A.d, note the validity is more restrictive than on ARMv7).
>> It's also not possible to use r15 in co-processor instructions.
> 
> Does this "Just Work" for arm32 Xen then?
> 
> What happens if an aarch32 guest accesses r15 in one of these ways on an
> aarch64 hypervisor? Is it verboten in ARM v8 or something else? E1.2.3
> describes it as deprecated, but not forbidden, but I suspect that "and the
> source address of the ldr is in MMIO space" is not covered there or
> something.

Based on the description of the ISS encoding for data abort, the
syndrome would be invalid.

Our data abort handler will inject a data abort to the guest for any
invalid instruction.

If we ever want to support the guest to write pc in an emulated MMIO, we
would have to decode the instruction.

> 
>> @@ -207,16 +214,42 @@ register_t *select_user_reg(struct cpu_user_regs
>> *regs, int reg)
>>      }
>>  #undef REGOFFS
>>  #else
>> -    /* In 64 bit the syndrome register contains the AArch64 register
>> -     * number even if the trap was from AArch32 mode. Except that
>> -     * AArch32 R15 (PC) is encoded as 0b11111.
>> +    /*
>> +     * On 64-bit the syndrome register contains the register index as
>> +     * viewed in AArch64 state even if the trap was from AArch32 mode.
>>       */
>> -    if ( reg == 0x1f /* && is aarch32 guest */)
>> -        return &regs->pc;
>>      return &regs->x0 + reg;
> 
> If reg were == 0x1f here then it would end up accessing regs->sp, which is
> only valid for traps from Xen to Xen (i.e. interrupt stack frames), whichif
> it were to somehow happen would lead to some rather interesting behaviour I
> fear. I'm pretty sure it can't happen, but I'd be happier with an explicit
> assert/BUG_ON.

I will had a BUG_ON Here.

> 
> Alternatively we could return NULL here to represent XZR and handle that
> appropriately in the {get,set}_user_reg, replacing the check for reg == 31 in 
> both places.

I would rather avoid select_user_regs to return NULL. The explicit
encoding check in {get,set}_user_reg is better.

Regards,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel