On 14/02/17 08:55, Jan Beulich wrote:
>>>> On 13.02.17 at 19:26, <andrew.coop...@citrix.com> wrote:
>> On 13/02/17 13:19, Jan Beulich wrote:
>>> ---
>>> TBD: Do we really want to re-init the TSS every time we are about to
>>> use it? This can happen quite often during boot, especially while
>>> grub is running.
>> The only case we need worry about is if the guest manually writes to the
>> area covered by the vm86 tss. I think, looking at the logic, that we
>> properly restore the old %tr when re-entering protected mode, so we
>> aren't at risk of having the vm86 tss on the leaving-side of a hardware
>> task switch.
> Yes. But that's the whole point of the question: The guest could
> equally well write to the TSS manually right after we've initialized
> it. And it only harms itself by doing so, hence we could as well
> init the TSS on a less frequently executed path.
Per this patch (I think) we initialise it each time the guest tries to
clear CR0.PE
Unless the VM86_TSS location is below the 1MB boundary, the guest can't
actually clobber it.
As an alternative, if you don't reinitialise each time, when would you
do so?
>
>> We have lasted thus far without, but given the issues recently
>> identified, the only safe conclusion to be drawn is that this
>> functionality hasn't been used in anger.
>>
>> For sensible performance, we need to keep the IO bitmap clear to avoid
>> taking the #GP emulation path.
>>
>> For correct behaviour, we need the entire software bitmap to be 0.
> This one is just for reasonable performance too, afaict. If faulting,
> just like with port accesses, we'd emulate the INT $nn.
Does that actually work? Upon re-injection of the event, won't hardware
follow the same bad path which caused the #GP fault to start with?
>
>>> +void hvm_prepare_vm86_tss(struct vcpu *v, uint32_t base, uint32_t limit)
>>> +{
>>> + /*
>>> + * If the provided area is large enough to cover at least the ISA port
>>> + * range, keep the bitmaps outside the base structure. For rather small
>>> + * areas (namely relevant for guests having been migrated from older
>>> + * Xen versions), maximize interrupt vector and port coverage by
>>> pointing
>>> + * the I/O bitmap at 0x20 (which puts the interrupt redirection bitmap
>>> + * right at zero), accepting accesses to port 0x235 (represented by
>>> bit 5
>>> + * of byte 0x46) to trigger #GP (which will simply result in the access
>>> + * being handled by the emulator via a slightly different path than it
>>> + * would be anyway). Be sure to include one extra byte at the end of
>>> the
>>> + * I/O bitmap (hence the missing "- 1" in the comparison is not an
>>> + * off-by-one mistake), which we deliberately don't fill with all ones.
>>> + */
>>> + uint16_t iomap = (limit >= sizeof(struct tss32) + (0x100 / 8) + (0x400
>>> / 8)
>>> + ? sizeof(struct tss32) : 0) + (0x100 / 8);
>>> +
>>> + ASSERT(limit >= sizeof(struct tss32) - 1);
>>> + hvm_copy_to_guest_phys(base, NULL, limit + 1, v);
>>> + hvm_copy_to_guest_phys(base + offsetof(struct tss32, iomap),
>>> + &iomap, sizeof(iomap), v);
>> This should be linear rather than phys.
> Strictly speaking yes, but since we're running with an ident map,
> it doesn't matter. And I'd prefer not to have to introduce a vcpu
> parameter to the linear copy function, as that would mean quite
> a few changes elsewhere. Would you be okay with me just
> attaching a respective comment here?
Ok.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
