Re: [Xen-devel] [PATCH v5 4/9] xen/x86: populate PVHv2 Dom0 physical memory map

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 27/01/17 16:40, Jan Beulich wrote:
>>>> On 27.01.17 at 14:20, <tim@xxxxxxx> wrote:
>> At 12:51 +0000 on 27 Jan (1485521470), Andrew Cooper wrote:
>>> On 27/01/17 11:14, Tim Deegan wrote:
>>>> But looking at it now, I'm not convinced of exactly how.  The magic
>>>> bitmap in the TSS is at [I/O Map Base Address] - 32, and the I/O map
>>>> base address itself lives at offset 100.  A zero'd TSS should mean an
>>>> I/O map at 0, and an interrupt redirection bitmap at -32, which would
>>>> plausibly work if the TSS were 256 bytes (matching the limit set in
>>>> Xen).  Perhaps it's only working because the 128 bytes following the
>>>> TSS in hvmloader happen to be zeros too?
>>> With an IO_base_map of 0, the software interrupt bitmap will end up
>>> being ahead of the TSS, not after it.
>> I should have thought that the segmented address calculation would
>> wrap and leave us at TSS + 224.
> I don't think wrapping takes the limit value into account. It's all
> linear address calculations, and as Andrew says the assumption
> in microcode likely is that things will be set up properly by any
> OS interested in using the interrupt bitmap.
>
>>>> I also don't remember why the TSS is 128 rather than 104 bytes.  The
>>>> SDM claims that the TSS must be larger than 104 bytes "when accessing
>>>> the I/O permission bit map or interrupt redirection bit map."
>>>> (7.2.2. "TSS Descriptor") but I suspect that just means that the
>>>> generated address of the bitmap must lie inside the limit.
>>> The documented way of expressing "no IO bitmap" is to set the map base
>>> to a value which exceeds the TSS limit.  All this means (I think) is
>>> that you must make a larger than default TSS if you want to use a IO or
>>> software interrupt bitmap.
>> Yes, I wonder about the I/O bitmap too.  We don't provide one, or even
>> enough space for a full one, but the current SDM is pretty clear that
>> the CPU will try to check it in virtual 8086 mode.
>>
>> It may be that all the ports actually used happen to fall in the 128
>> bytes of zeros that we provide.
> I suppose so: This is precisely enough for the ISA port range.
>
> So what we'll need to do then, as I understand it from the
> discussion so far:
>
> - vmx_set_segment_register() will need to set a correct limit
> - vmx_set_segment_register() should initialize the TSS every
>   time (including setting the I/O bitmap address to no lower
>   than 32)
> - hvmloader's init_vm86_tss() will need to allocate 160 bytes
>   rather than 128 (and we should expose this number, so that
>   Roger can also use it)
>
> Perhaps we should even introduce a hypercall for hvmloader
> to query the needed value, rather than exposing a hardcoded
> number?

I suggest we remove all responsibility of managing this from hvmloader. 
The only thing hvmloader does is allocate space for it, and reserve it
in the E820.

It is conceptually related to IDENT_PT, although the IDENT_PT must be
allocated and filled in by the domain builder for the HVM guest to
function.  It would be cleaner for the domain builder to also allocate
an adjacent page for the VM86_TSS when it constructs the IDENT_PT.

All HVMLoader needs to do is read the two hvmparams and adjust the E820
table suitably.

Finally, the IO bitmap needs to be a fraction larger than 160 bytes.

From tools/firmware/rombios/rombios.h:

#define PANIC_PORT  0x400
#define PANIC_PORT2 0x401
#define INFO_PORT   0x402
#define DEBUG_PORT  0x403

which are just above the ISA range.  I'd also just allocate a full page
for it; no OS is going to bother trying to use fractions of a page
around an E820 reserved region.

~Andrew

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