[PATCH] x86/HVM: support IOMMU-related Viridian CPUID bits

[PATCH] x86/HVM: support IOMMU-related Viridian CPUID bits

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 884 bytes --]

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/hvm/viridian.c
+++ b/xen/arch/x86/hvm/viridian.c
@@ -50,6 +50,8 @@
 #define CPUID6A_APIC_OVERLAY    (1 << 0)
 #define CPUID6A_MSR_BITMAPS     (1 << 1)
 #define CPUID6A_NESTED_PAGING   (1 << 3)
+#define CPUID6A_DMA_REMAP       (1 << 4)
+#define CPUID6A_INTR_REMAP      (1 << 5)
 
 int cpuid_viridian_leaves(unsigned int leaf, unsigned int *eax,
                           unsigned int *ebx, unsigned int *ecx,
@@ -111,6 +113,12 @@ int cpuid_viridian_leaves(unsigned int l
             *eax |= CPUID6A_MSR_BITMAPS;
         if ( hap_enabled(d) )
             *eax |= CPUID6A_NESTED_PAGING;
+        if ( iommu_enabled )
+        {
+            *eax |= CPUID6A_DMA_REMAP;
+            if ( iommu_intremap )
+                *eax |= CPUID6A_INTR_REMAP;
+        }
         break;
     }
 




[-- Attachment #2: x86-viridian-CPUID-leaf-6-IOMMU.patch --]
[-- Type: text/plain, Size: 932 bytes --]

x86/HVM: support IOMMU-related Viridian CPUID bits

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/hvm/viridian.c
+++ b/xen/arch/x86/hvm/viridian.c
@@ -50,6 +50,8 @@
 #define CPUID6A_APIC_OVERLAY    (1 << 0)
 #define CPUID6A_MSR_BITMAPS     (1 << 1)
 #define CPUID6A_NESTED_PAGING   (1 << 3)
+#define CPUID6A_DMA_REMAP       (1 << 4)
+#define CPUID6A_INTR_REMAP      (1 << 5)
 
 int cpuid_viridian_leaves(unsigned int leaf, unsigned int *eax,
                           unsigned int *ebx, unsigned int *ecx,
@@ -111,6 +113,12 @@ int cpuid_viridian_leaves(unsigned int l
             *eax |= CPUID6A_MSR_BITMAPS;
         if ( hap_enabled(d) )
             *eax |= CPUID6A_NESTED_PAGING;
+        if ( iommu_enabled )
+        {
+            *eax |= CPUID6A_DMA_REMAP;
+            if ( iommu_intremap )
+                *eax |= CPUID6A_INTR_REMAP;
+        }
         break;
     }
 

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH] x86/HVM: support IOMMU-related Viridian CPUID bits

[Paul Durrant]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: 01 August 2014 14:49
> To: xen-devel
> Cc: Andrew Cooper; Paul Durrant; Keir (Xen.org)
> Subject: [PATCH] x86/HVM: support IOMMU-related Viridian CPUID bits
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Whilst this patch is technically fine, is it of any real use? From my reading of the Hypervisor Top Level Functional Spec (v4.0a) these bits are only of relevance to the root partition.
If they do make a difference to a guest, should we not also be specifying bits 6 and 7 in leaf 40000004:EAX and putting a value in 40000005:ECX?

  Paul

> 
> --- a/xen/arch/x86/hvm/viridian.c
> +++ b/xen/arch/x86/hvm/viridian.c
> @@ -50,6 +50,8 @@
>  #define CPUID6A_APIC_OVERLAY    (1 << 0)
>  #define CPUID6A_MSR_BITMAPS     (1 << 1)
>  #define CPUID6A_NESTED_PAGING   (1 << 3)
> +#define CPUID6A_DMA_REMAP       (1 << 4)
> +#define CPUID6A_INTR_REMAP      (1 << 5)
> 
>  int cpuid_viridian_leaves(unsigned int leaf, unsigned int *eax,
>                            unsigned int *ebx, unsigned int *ecx,
> @@ -111,6 +113,12 @@ int cpuid_viridian_leaves(unsigned int l
>              *eax |= CPUID6A_MSR_BITMAPS;
>          if ( hap_enabled(d) )
>              *eax |= CPUID6A_NESTED_PAGING;
> +        if ( iommu_enabled )
> +        {
> +            *eax |= CPUID6A_DMA_REMAP;
> +            if ( iommu_intremap )
> +                *eax |= CPUID6A_INTR_REMAP;
> +        }
>          break;
>      }
> 
> 
> 

Re: [PATCH] x86/HVM: support IOMMU-related Viridian CPUID bits

[Jan Beulich]

>>> On 01.08.14 at 15:58, <Paul.Durrant@citrix.com> wrote:
>>  -----Original Message-----
>> From: Jan Beulich [mailto:JBeulich@suse.com]
>> Sent: 01 August 2014 14:49
>> To: xen-devel
>> Cc: Andrew Cooper; Paul Durrant; Keir (Xen.org)
>> Subject: [PATCH] x86/HVM: support IOMMU-related Viridian CPUID bits
>> 
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Whilst this patch is technically fine, is it of any real use? From my 
> reading of the Hypervisor Top Level Functional Spec (v4.0a) these bits are 
> only of relevance to the root partition.

I'm not that familiar with Hyper-V concepts, so I'm not sure what "root
partition" refers to. If it was what I imagined (the host OS instance),
then these bits clearly couldn't be meant for it, as it only sees the
native CPUID output.

> If they do make a difference to a guest, should we not also be specifying 
> bits 6 and 7 in leaf 40000004:EAX

Rather not I would say, at least on VT-d, until we have a vIOMMU.
And I'm not sure the vIOMMU on the AMD side is mature enough
to _recommend_ its use.

> and putting a value in 40000005:ECX?

That one I'd view as separate work item, aiming at populating its
EAX and EBX outputs at once. Albeit I may again have a terminology
issue here: For a guest, what's the difference between virtual and
logical CPUs? Additionally, for ECX I don't think we would know the
correct value to put there under Xen, since we're not doing the
remapping in a vector centric way (i.e. this value might be reflecting
some Hyper-V internals).

Jan

Re: [PATCH] x86/HVM: support IOMMU-related Viridian CPUID bits

[Paul Durrant]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: 01 August 2014 15:16
> To: Paul Durrant
> Cc: Andrew Cooper; xen-devel; Keir (Xen.org)
> Subject: RE: [PATCH] x86/HVM: support IOMMU-related Viridian CPUID bits
> 
> >>> On 01.08.14 at 15:58, <Paul.Durrant@citrix.com> wrote:
> >>  -----Original Message-----
> >> From: Jan Beulich [mailto:JBeulich@suse.com]
> >> Sent: 01 August 2014 14:49
> >> To: xen-devel
> >> Cc: Andrew Cooper; Paul Durrant; Keir (Xen.org)
> >> Subject: [PATCH] x86/HVM: support IOMMU-related Viridian CPUID bits
> >>
> >> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> >
> > Whilst this patch is technically fine, is it of any real use? From my
> > reading of the Hypervisor Top Level Functional Spec (v4.0a) these bits are
> > only of relevance to the root partition.
> 
> I'm not that familiar with Hyper-V concepts, so I'm not sure what "root
> partition" refers to. If it was what I imagined (the host OS instance),
> then these bits clearly couldn't be meant for it, as it only sees the
> native CPUID output.
> 

I'm not sure that's true. I think Hyper-V runs its root partition (i.e. control domain) in a VM container and so it doesn't see native CPUID either. Anyway, what is the benefit of adding these two bits in Xen's viridian code? Have they actually been observed to make a difference to a Windows guest?

  Paul

> > If they do make a difference to a guest, should we not also be specifying
> > bits 6 and 7 in leaf 40000004:EAX
> 
> Rather not I would say, at least on VT-d, until we have a vIOMMU.
> And I'm not sure the vIOMMU on the AMD side is mature enough
> to _recommend_ its use.
> 
> > and putting a value in 40000005:ECX?
> 
> That one I'd view as separate work item, aiming at populating its
> EAX and EBX outputs at once. Albeit I may again have a terminology
> issue here: For a guest, what's the difference between virtual and
> logical CPUs? Additionally, for ECX I don't think we would know the
> correct value to put there under Xen, since we're not doing the
> remapping in a vector centric way (i.e. this value might be reflecting
> some Hyper-V internals).
> 
> Jan

Re: [PATCH] x86/HVM: support IOMMU-related Viridian CPUID bits

[Jan Beulich]

>>> On 01.08.14 at 16:42, <Paul.Durrant@citrix.com> wrote:
>>  -----Original Message-----
>> From: Jan Beulich [mailto:JBeulich@suse.com]
>> Sent: 01 August 2014 15:16
>> To: Paul Durrant
>> Cc: Andrew Cooper; xen-devel; Keir (Xen.org)
>> Subject: RE: [PATCH] x86/HVM: support IOMMU-related Viridian CPUID bits
>> 
>> >>> On 01.08.14 at 15:58, <Paul.Durrant@citrix.com> wrote:
>> >>  -----Original Message-----
>> >> From: Jan Beulich [mailto:JBeulich@suse.com]
>> >> Sent: 01 August 2014 14:49
>> >> To: xen-devel
>> >> Cc: Andrew Cooper; Paul Durrant; Keir (Xen.org)
>> >> Subject: [PATCH] x86/HVM: support IOMMU-related Viridian CPUID bits
>> >>
>> >> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> >
>> > Whilst this patch is technically fine, is it of any real use? From my
>> > reading of the Hypervisor Top Level Functional Spec (v4.0a) these bits are
>> > only of relevance to the root partition.
>> 
>> I'm not that familiar with Hyper-V concepts, so I'm not sure what "root
>> partition" refers to. If it was what I imagined (the host OS instance),
>> then these bits clearly couldn't be meant for it, as it only sees the
>> native CPUID output.
>> 
> 
> I'm not sure that's true. I think Hyper-V runs its root partition (i.e. 
> control domain) in a VM container and so it doesn't see native CPUID either. 
> Anyway, what is the benefit of adding these two bits in Xen's viridian code? 
> Have they actually been observed to make a difference to a Windows guest?

Not that I know of. It just seems odd not to populate bits we can
populate, simply _assuming_ that the Windows guys would not have
added them without reason (i.e. _they_ know they can do whatever
it is better with that knowledge).

Jan

Re: [PATCH] x86/HVM: support IOMMU-related Viridian CPUID bits

[Paul Durrant]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: 01 August 2014 15:57
> To: Paul Durrant
> Cc: Andrew Cooper; xen-devel; Keir (Xen.org)
> Subject: RE: [PATCH] x86/HVM: support IOMMU-related Viridian CPUID bits
> 
> >>> On 01.08.14 at 16:42, <Paul.Durrant@citrix.com> wrote:
> >>  -----Original Message-----
> >> From: Jan Beulich [mailto:JBeulich@suse.com]
> >> Sent: 01 August 2014 15:16
> >> To: Paul Durrant
> >> Cc: Andrew Cooper; xen-devel; Keir (Xen.org)
> >> Subject: RE: [PATCH] x86/HVM: support IOMMU-related Viridian CPUID
> bits
> >>
> >> >>> On 01.08.14 at 15:58, <Paul.Durrant@citrix.com> wrote:
> >> >>  -----Original Message-----
> >> >> From: Jan Beulich [mailto:JBeulich@suse.com]
> >> >> Sent: 01 August 2014 14:49
> >> >> To: xen-devel
> >> >> Cc: Andrew Cooper; Paul Durrant; Keir (Xen.org)
> >> >> Subject: [PATCH] x86/HVM: support IOMMU-related Viridian CPUID
> bits
> >> >>
> >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> >> >
> >> > Whilst this patch is technically fine, is it of any real use? From my
> >> > reading of the Hypervisor Top Level Functional Spec (v4.0a) these bits
> are
> >> > only of relevance to the root partition.
> >>
> >> I'm not that familiar with Hyper-V concepts, so I'm not sure what "root
> >> partition" refers to. If it was what I imagined (the host OS instance),
> >> then these bits clearly couldn't be meant for it, as it only sees the
> >> native CPUID output.
> >>
> >
> > I'm not sure that's true. I think Hyper-V runs its root partition (i.e.
> > control domain) in a VM container and so it doesn't see native CPUID
> either.
> > Anyway, what is the benefit of adding these two bits in Xen's viridian code?
> > Have they actually been observed to make a difference to a Windows
> guest?
> 
> Not that I know of. It just seems odd not to populate bits we can
> populate, simply _assuming_ that the Windows guys would not have
> added them without reason (i.e. _they_ know they can do whatever
> it is better with that knowledge).
> 

Maybe. The spec. certainly implies that no Xen guest is ever likely to do anything useful with them though. I'm also concerned about these bits suddenly becoming visible across a migrate.

  Paul

Re: [PATCH] x86/HVM: support IOMMU-related Viridian CPUID bits

[Jan Beulich]

>>> On 01.08.14 at 17:01, <Paul.Durrant@citrix.com> wrote:
> Maybe. The spec. certainly implies that no Xen guest is ever likely to do 
> anything useful with them though. I'm also concerned about these bits 
> suddenly becoming visible across a migrate.

Or disappear. But that's a problem with CPUID4A_MSR_BASED_APIC,
CPUID6A_APIC_OVERLAY, and CPUID6A_MSR_BITMAPS too. Do you
suggest we don't report any CPUID bits depending on some condition
which may change across migration, until proper validation of feature
parity of both hosts is in place?

Jan

Re: [PATCH] x86/HVM: support IOMMU-related Viridian CPUID bits

[Paul Durrant]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: 01 August 2014 16:20
> To: Paul Durrant
> Cc: Andrew Cooper; xen-devel; Keir (Xen.org)
> Subject: RE: [PATCH] x86/HVM: support IOMMU-related Viridian CPUID bits
> 
> >>> On 01.08.14 at 17:01, <Paul.Durrant@citrix.com> wrote:
> > Maybe. The spec. certainly implies that no Xen guest is ever likely to do
> > anything useful with them though. I'm also concerned about these bits
> > suddenly becoming visible across a migrate.
> 
> Or disappear. But that's a problem with CPUID4A_MSR_BASED_APIC,
> CPUID6A_APIC_OVERLAY, and CPUID6A_MSR_BITMAPS too. Do you
> suggest we don't report any CPUID bits depending on some condition
> which may change across migration, until proper validation of feature
> parity of both hosts is in place?
> 

It's also a problem with the TSC/APIC frequency features too - a migration from one version of Xen to another can make them magically appear.
For 4A bits, I think we should make sure stuff shouldn't change after boot. For 6A bits, having re-read the comment in the spec. it sounds like it may be ok:

"0x40000006: Implementation hardware features. Indicates which hardware-specific features have been detected and are currently in use by the hypervisor"

There's nothing to say they can't change, but then there's nothing to say they can.

I'm working on a patch to add reference TSC page support at the moment, which is dependent on invariant TSC, and the spec. does have provision for that feature disappearing on migrate... so maybe we are ok.

  paul

Re: [PATCH] x86/HVM: support IOMMU-related Viridian CPUID bits

[George Dunlap]

On Fri, Aug 1, 2014 at 10:57 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 01.08.14 at 16:42, <Paul.Durrant@citrix.com> wrote:
>>>  -----Original Message-----
>>> From: Jan Beulich [mailto:JBeulich@suse.com]
>>> Sent: 01 August 2014 15:16
>>> To: Paul Durrant
>>> Cc: Andrew Cooper; xen-devel; Keir (Xen.org)
>>> Subject: RE: [PATCH] x86/HVM: support IOMMU-related Viridian CPUID bits
>>>
>>> >>> On 01.08.14 at 15:58, <Paul.Durrant@citrix.com> wrote:
>>> >>  -----Original Message-----
>>> >> From: Jan Beulich [mailto:JBeulich@suse.com]
>>> >> Sent: 01 August 2014 14:49
>>> >> To: xen-devel
>>> >> Cc: Andrew Cooper; Paul Durrant; Keir (Xen.org)
>>> >> Subject: [PATCH] x86/HVM: support IOMMU-related Viridian CPUID bits
>>> >>
>>> >> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>> >
>>> > Whilst this patch is technically fine, is it of any real use? From my
>>> > reading of the Hypervisor Top Level Functional Spec (v4.0a) these bits are
>>> > only of relevance to the root partition.
>>>
>>> I'm not that familiar with Hyper-V concepts, so I'm not sure what "root
>>> partition" refers to. If it was what I imagined (the host OS instance),
>>> then these bits clearly couldn't be meant for it, as it only sees the
>>> native CPUID output.
>>>
>>
>> I'm not sure that's true. I think Hyper-V runs its root partition (i.e.
>> control domain) in a VM container and so it doesn't see native CPUID either.
>> Anyway, what is the benefit of adding these two bits in Xen's viridian code?
>> Have they actually been observed to make a difference to a Windows guest?
>
> Not that I know of. It just seems odd not to populate bits we can
> populate, simply _assuming_ that the Windows guys would not have
> added them without reason (i.e. _they_ know they can do whatever
> it is better with that knowledge).

Does Hyper-V actually expose these bits to non-root partitions (i.e.,
the viridian equivalent of domUs)?

 -George

Re: [PATCH] x86/HVM: support IOMMU-related Viridian CPUID bits

[Paul Durrant]

> -----Original Message-----
> From: dunlapg@gmail.com [mailto:dunlapg@gmail.com] On Behalf Of
> George Dunlap
> Sent: 04 August 2014 17:30
> To: Jan Beulich
> Cc: Paul Durrant; Andrew Cooper; Keir (Xen.org); xen-devel
> Subject: Re: [Xen-devel] [PATCH] x86/HVM: support IOMMU-related Viridian
> CPUID bits
> 
> On Fri, Aug 1, 2014 at 10:57 AM, Jan Beulich <JBeulich@suse.com> wrote:
> >>>> On 01.08.14 at 16:42, <Paul.Durrant@citrix.com> wrote:
> >>>  -----Original Message-----
> >>> From: Jan Beulich [mailto:JBeulich@suse.com]
> >>> Sent: 01 August 2014 15:16
> >>> To: Paul Durrant
> >>> Cc: Andrew Cooper; xen-devel; Keir (Xen.org)
> >>> Subject: RE: [PATCH] x86/HVM: support IOMMU-related Viridian CPUID
> bits
> >>>
> >>> >>> On 01.08.14 at 15:58, <Paul.Durrant@citrix.com> wrote:
> >>> >>  -----Original Message-----
> >>> >> From: Jan Beulich [mailto:JBeulich@suse.com]
> >>> >> Sent: 01 August 2014 14:49
> >>> >> To: xen-devel
> >>> >> Cc: Andrew Cooper; Paul Durrant; Keir (Xen.org)
> >>> >> Subject: [PATCH] x86/HVM: support IOMMU-related Viridian CPUID
> bits
> >>> >>
> >>> >> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> >>> >
> >>> > Whilst this patch is technically fine, is it of any real use? From my
> >>> > reading of the Hypervisor Top Level Functional Spec (v4.0a) these bits
> are
> >>> > only of relevance to the root partition.
> >>>
> >>> I'm not that familiar with Hyper-V concepts, so I'm not sure what "root
> >>> partition" refers to. If it was what I imagined (the host OS instance),
> >>> then these bits clearly couldn't be meant for it, as it only sees the
> >>> native CPUID output.
> >>>
> >>
> >> I'm not sure that's true. I think Hyper-V runs its root partition (i.e.
> >> control domain) in a VM container and so it doesn't see native CPUID
> either.
> >> Anyway, what is the benefit of adding these two bits in Xen's viridian
> code?
> >> Have they actually been observed to make a difference to a Windows
> guest?
> >
> > Not that I know of. It just seems odd not to populate bits we can
> > populate, simply _assuming_ that the Windows guys would not have
> > added them without reason (i.e. _they_ know they can do whatever
> > it is better with that knowledge).
> 
> Does Hyper-V actually expose these bits to non-root partitions (i.e.,
> the viridian equivalent of domUs)?
> 

I guess someone would have to try it. The spec. is not clear.

The features are documented to only be useful to the 'root partition', although from looking at the 'access denied' error documentation for interrupt remapping... it says the partition must possess the CpuManagement privilege. Privileges are advertised to the partitions via CPUID:40000003:EBX (CpuManagement being bit 12), and Xen has that register hardcoded to 0. So, I guess there's probably no harm in setting the bits but I doubt they are of any value since we never give any guest privilege to use them.

  Paul

>  -George