Re: [Xen-devel] [PATCH v8 15/15] xen: add new Xen cpuid node for max address width info

[Juergen Gross <jgross@xxxxxxxx>]

On 20/09/17 14:18, Jan Beulich wrote:
>>>> On 20.09.17 at 08:34, <jgross@xxxxxxxx> wrote:
>> On very large hosts a guest needs to know whether it will have to
> 
> ... a PV guest ...

What about a HVM guest with (potentially) more than 16TB?

>> handle frame numbers larger than 32 bits in order to select the
>> appropriate grant interface version.
>>
>> Add a new Xen specific CPUID node to contain the maximum guest address
>> width
> 
> "guest address width" is ambiguous here, the more when looking at
> what you actually return. We should no longer allow ourselves to
> mix up the different address spaces.

I've chosen "guest address width" similar to the "guest frame number"
we already have: it is MFN based for PV and PFN based for HVM (and ARM).
I'm open for a better name.

> The limit you want to report
> here is that in MFN space, which ought to be of no relevance to
> HVM guests. Therefore I'm against uniformly exposing this (as much
> as almost no other host property should have any relevance for
> HVM guests), and would instead like to see a PV-only leaf just like
> we already have a HVM only one.

As said above: a HVM guest needs to know whether it will have to deal
with frame numbers >32 bits, too.

For HVM guests this would just be a hint that the host might be large
enough for this to happen, as even today a HVM guest could in theory
reorganize its memory map to have parts of the memory above the 16TB
border even with only rather small amounts of memory. But this would
be the problem of the guest then.

For the future I could envision a per-domain parameter setting the
upper limit where a guest could put its memory. This parameter would
then influence the new CPUID value fro HVM domains, of course.

> 
>> --- a/xen/arch/x86/traps.c
>> +++ b/xen/arch/x86/traps.c
>> @@ -929,6 +929,10 @@ void cpuid_hypervisor_leaves(const struct vcpu *v, 
>> uint32_t leaf,
>>          res->b = v->vcpu_id;
>>          break;
>>  
>> +    case 5: /* Host specific parameters */
>> +        res->a = generic_flsl(get_upper_mfn_bound() - 1) + PAGE_SHIFT;
>> +        break;
> 
> Already when looking at the patch introducing the function I was
> wondering whether the function wouldn't better return the highest
> frame number instead of the first invalid one. From an abstract
> perspective this would allow an arch (e.g. ARM32) to report that
> all addresses are valid.

That's fine with me.

>> --- a/xen/include/public/arch-x86/cpuid.h
>> +++ b/xen/include/public/arch-x86/cpuid.h
>> @@ -85,6 +85,15 @@
>>  #define XEN_HVM_CPUID_IOMMU_MAPPINGS   (1u << 2)
>>  #define XEN_HVM_CPUID_VCPU_ID_PRESENT  (1u << 3) /* vcpu id is present in 
>> EBX 
>> */
>>  
>> -#define XEN_CPUID_MAX_NUM_LEAVES 4
>> +/*
>> + * Leaf 6 (0x40000x05)
>> + * Host specific parameters
>> + * EAX: bits 0-7: max guest address width
>> + */
>> +
>> +/* Max. address width in bits taking memory hotplug into account. */
>> +#define XEN_CPUID_GUEST_ADDRESS_WIDTH_MASK (255u << 0)
> 
> Please use hex numbers for multi-digit masks, and please make
> explicit (at least in the comment, perhaps also in the constant's
> name) which "address" is being talked about here.

Okay.


Juergen

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