Re: [Xen-devel] [PATCH for 4.9 6/6] x86/emul: Require callers to provide LMA in the emulation context

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 05.04.17 at 18:24, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 03/04/17 11:10, Jan Beulich wrote:
>>>>> On 31.03.17 at 21:50, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> --- a/xen/arch/x86/mm.c
>>> +++ b/xen/arch/x86/mm.c
>>> @@ -5410,6 +5410,7 @@ int ptwr_do_page_fault(struct vcpu *v, unsigned long 
>>> addr,
>>>          .ctxt = {
>>>              .regs = regs,
>>>              .vendor = d->arch.cpuid->x86_vendor,
>>> +            .lma = true,
>>>              .addr_size = is_pv_32bit_domain(d) ? 32 : BITS_PER_LONG,
>>>              .sp_size   = is_pv_32bit_domain(d) ? 32 : BITS_PER_LONG,
>>>          },
>>> @@ -5564,6 +5565,7 @@ int mmio_ro_do_page_fault(struct vcpu *v, unsigned 
>>> long addr,
>>>      struct x86_emulate_ctxt ctxt = {
>>>          .regs = regs,
>>>          .vendor = v->domain->arch.cpuid->x86_vendor,
>>> +        .lma = true,
>> Hmm, both of these are correct from Xen's pov, but potentially
>> wrong from the guest's. Since system segments aren't being
>> dealt with here, I think this difference is benign, but I think it
>> warrants at least a comment. If we ever meant to emulate
>> LLDT, this would become at active problem, as the guest's view
>> on gate descriptor layout would differ from that resulting from
>> setting .lma to true here. Same for emulate_privileged_op() then.
> 
> As discovered in the meantime, things like LLDT/LTR and call gates are
> far more complicated.
> 
> Still, setting LMA to true here is the right thing to do, as it is an
> accurate statement of processor state.  Despite the level of
> compatibility for 32bit, a 32bit PV guest isn't entirely isolated from
> the fact that Xen is 64bit.

Yes, but still call gates (which we don't currently handle in the
emulator itself) require 32-bit treatment for 32-bit guests, so
setting lma to true would still seem wrong. And the value of lma
is, as we now know, irrelevant for LDT and TSS descriptors (I'm
about to verify AMD behaves identically to Intel here).

>>> --- a/xen/arch/x86/x86_emulate/x86_emulate.h
>>> +++ b/xen/arch/x86/x86_emulate/x86_emulate.h
>>> @@ -457,6 +457,9 @@ struct x86_emulate_ctxt
>>>      /* Set this if writes may have side effects. */
>>>      bool force_writeback;
>>>  
>>> +    /* Long mode active? */
>>> +    bool lma;
>> I don't think this should be in the input only section, as an insn
>> can have the effect of clearing it.
>>
>> Also should x86_emulate_wrapper() perhaps assert the field to
>> be false for 32-bit (test harness) builds?
> 
> I'd prefer not to.  There is nothing in principle wrong with trying to
> feed a 32bit compatibility segment through a 32bit build of the test
> harness.

Actually I've just figured myself that such an assertion would better
be checking that mode_64bit() implies lma to be set.

Jan


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