thr3ads.net - Xen devel - [Xen-devel] [PATCH] VT-d: don''t reject possibly valid DRHD or RMRR [Oct 2011]

If this information is useful, please help other people find it:
Share via:

Jan Beulich

2011-Oct-04 11:30 UTC

[Xen-devel] [PATCH] VT-d: don''t reject possibly valid DRHD or RMRR

If a non-zero PCI segment isn''t accessible during Xen boot (because
firmware decided to not enter the necessary MMIO space into the E820
table), devices referred to on those segments through DRHD or RMRR
structures should not be rejected just because the devices can''t be
found.

This is in line with what is being done in at least one other case
already: Systems with more than one PCI segment (usually high end
ones) are assumed to have valid firmware provided data, while systems
with just segment 0 continue to have their firmware tables validated.

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

--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -53,6 +53,11 @@ static inline struct pci_seg *get_pseg(u
     return radix_tree_lookup(&pci_segments, seg);
 }
 
+bool_t pci_known_segment(u16 seg)
+{
+    return get_pseg(seg) != NULL;
+}
+
 static struct pci_seg *alloc_pseg(u16 seg)
 {
     struct pci_seg *pseg = get_pseg(seg);
--- a/xen/drivers/passthrough/vtd/dmar.c
+++ b/xen/drivers/passthrough/vtd/dmar.c
@@ -444,10 +444,14 @@ acpi_parse_one_drhd(struct acpi_dmar_ent
     else
     {
         u8 b, d, f;
-        int i, invalid_cnt = 0;
+        unsigned int i = 0, invalid_cnt = 0;
         void *p;
 
-        for ( i = 0, p = dev_scope_start; i < dmaru->scope.devices_cnt;
+        /* Skip checking if segment is not accessible yet. */
+        if ( !pci_known_segment(drhd->segment) )
+            i = UINT_MAX;
+
+        for ( p = dev_scope_start; i < dmaru->scope.devices_cnt;
               i++, p += ((struct acpi_dev_scope *)p)->length )
         {
             if ( ((struct acpi_dev_scope *)p)->dev_type == ACPI_DEV_IOAPIC
||
@@ -549,7 +553,12 @@ acpi_parse_one_rmrr(struct acpi_dmar_ent
     else
     {
         u8 b, d, f;
-        int i, ignore = 0;
+        bool_t ignore = 0;
+        unsigned int i = 0;
+
+        /* Skip checking if segment is not accessible yet. */
+        if ( !pci_known_segment(rmrr->segment) )
+            i = UINT_MAX;
 
         for ( i = 0; i < rmrru->scope.devices_cnt; i++ )
         {
--- a/xen/include/xen/pci.h
+++ b/xen/include/xen/pci.h
@@ -82,6 +82,7 @@ enum {
     DEV_TYPE_PCI,
 };
 
+bool_t pci_known_segment(u16 seg);
 int pci_device_detect(u16 seg, u8 bus, u8 dev, u8 func);
 int scan_pci_devices(void);
 int pdev_type(u16 seg, u8 bus, u8 devfn);




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

Kay, Allen M

2011-Oct-07 01:51 UTC

head link

[Xen-devel] RE: [PATCH] VT-d: don''t reject possibly valid DRHD or RMRR

For RMRR case, looks like you miss the following change (or something similar):

-for ( i = 0; i < rmrru->scope.devices_cnt; i++ )
+for (; i < rmrru->scope.devices_cnt; i++ )

Otherwise, the logic for handling non-zero PCI segments looks reasonable.

Allen

-----Original Message-----
From: Jan Beulich [mailto:JBeulich@suse.com] 
Sent: Tuesday, October 04, 2011 4:30 AM
To: xen-devel@lists.xensource.com
Cc: Kay, Allen M
Subject: [PATCH] VT-d: don''t reject possibly valid DRHD or RMRR

If a non-zero PCI segment isn''t accessible during Xen boot (because
firmware decided to not enter the necessary MMIO space into the E820
table), devices referred to on those segments through DRHD or RMRR
structures should not be rejected just because the devices can''t be
found.

This is in line with what is being done in at least one other case
already: Systems with more than one PCI segment (usually high end
ones) are assumed to have valid firmware provided data, while systems
with just segment 0 continue to have their firmware tables validated.

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

--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -53,6 +53,11 @@ static inline struct pci_seg *get_pseg(u
     return radix_tree_lookup(&pci_segments, seg);
 }
 
+bool_t pci_known_segment(u16 seg)
+{
+    return get_pseg(seg) != NULL;
+}
+
 static struct pci_seg *alloc_pseg(u16 seg)
 {
     struct pci_seg *pseg = get_pseg(seg);
--- a/xen/drivers/passthrough/vtd/dmar.c
+++ b/xen/drivers/passthrough/vtd/dmar.c
@@ -444,10 +444,14 @@ acpi_parse_one_drhd(struct acpi_dmar_ent
     else
     {
         u8 b, d, f;
-        int i, invalid_cnt = 0;
+        unsigned int i = 0, invalid_cnt = 0;
         void *p;
 
-        for ( i = 0, p = dev_scope_start; i < dmaru->scope.devices_cnt;
+        /* Skip checking if segment is not accessible yet. */
+        if ( !pci_known_segment(drhd->segment) )
+            i = UINT_MAX;
+
+        for ( p = dev_scope_start; i < dmaru->scope.devices_cnt;
               i++, p += ((struct acpi_dev_scope *)p)->length )
         {
             if ( ((struct acpi_dev_scope *)p)->dev_type == ACPI_DEV_IOAPIC
||
@@ -549,7 +553,12 @@ acpi_parse_one_rmrr(struct acpi_dmar_ent
     else
     {
         u8 b, d, f;
-        int i, ignore = 0;
+        bool_t ignore = 0;
+        unsigned int i = 0;
+
+        /* Skip checking if segment is not accessible yet. */
+        if ( !pci_known_segment(rmrr->segment) )
+            i = UINT_MAX;
 
         for ( i = 0; i < rmrru->scope.devices_cnt; i++ )
         {
--- a/xen/include/xen/pci.h
+++ b/xen/include/xen/pci.h
@@ -82,6 +82,7 @@ enum {
     DEV_TYPE_PCI,
 };
 
+bool_t pci_known_segment(u16 seg);
 int pci_device_detect(u16 seg, u8 bus, u8 dev, u8 func);
 int scan_pci_devices(void);
 int pdev_type(u16 seg, u8 bus, u8 devfn);




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

Jan Beulich

2011-Oct-07 07:17 UTC

head link

[Xen-devel] RE: [PATCH] VT-d: don''t reject possibly valid DRHD or RMRR

>>> On 07.10.11 at 03:51, "Kay, Allen M"
<allen.m.kay@intel.com> wrote:
> For RMRR case, looks like you miss the following change (or something 
> similar):
> 
> -for ( i = 0; i < rmrru->scope.devices_cnt; i++ )
> +for (; i < rmrru->scope.devices_cnt; i++ )
Indeed.
> Otherwise, the logic for handling non-zero PCI segments looks reasonable.
So do I take this as an acked-by-with-above-change?
> Allen
> 
> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com] 
> Sent: Tuesday, October 04, 2011 4:30 AM
> To: xen-devel@lists.xensource.com 
> Cc: Kay, Allen M
> Subject: [PATCH] VT-d: don''t reject possibly valid DRHD or RMRR
> 
> If a non-zero PCI segment isn''t accessible during Xen boot
(because
> firmware decided to not enter the necessary MMIO space into the E820
> table), devices referred to on those segments through DRHD or RMRR
> structures should not be rejected just because the devices can''t
be
> found.
> 
> This is in line with what is being done in at least one other case
> already: Systems with more than one PCI segment (usually high end
> ones) are assumed to have valid firmware provided data, while systems
> with just segment 0 continue to have their firmware tables validated.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> --- a/xen/drivers/passthrough/pci.c
> +++ b/xen/drivers/passthrough/pci.c
> @@ -53,6 +53,11 @@ static inline struct pci_seg *get_pseg(u
>      return radix_tree_lookup(&pci_segments, seg);
>  }
>  
> +bool_t pci_known_segment(u16 seg)
> +{
> +    return get_pseg(seg) != NULL;
> +}
> +
>  static struct pci_seg *alloc_pseg(u16 seg)
>  {
>      struct pci_seg *pseg = get_pseg(seg);
> --- a/xen/drivers/passthrough/vtd/dmar.c
> +++ b/xen/drivers/passthrough/vtd/dmar.c
> @@ -444,10 +444,14 @@ acpi_parse_one_drhd(struct acpi_dmar_ent
>      else
>      {
>          u8 b, d, f;
> -        int i, invalid_cnt = 0;
> +        unsigned int i = 0, invalid_cnt = 0;
>          void *p;
>  
> -        for ( i = 0, p = dev_scope_start; i <
dmaru->scope.devices_cnt;
> +        /* Skip checking if segment is not accessible yet. */
> +        if ( !pci_known_segment(drhd->segment) )
> +            i = UINT_MAX;
> +
> +        for ( p = dev_scope_start; i < dmaru->scope.devices_cnt;
>                i++, p += ((struct acpi_dev_scope *)p)->length )
>          {
>              if ( ((struct acpi_dev_scope *)p)->dev_type ==
ACPI_DEV_IOAPIC ||
> @@ -549,7 +553,12 @@ acpi_parse_one_rmrr(struct acpi_dmar_ent
>      else
>      {
>          u8 b, d, f;
> -        int i, ignore = 0;
> +        bool_t ignore = 0;
> +        unsigned int i = 0;
> +
> +        /* Skip checking if segment is not accessible yet. */
> +        if ( !pci_known_segment(rmrr->segment) )
> +            i = UINT_MAX;
>  
>          for ( i = 0; i < rmrru->scope.devices_cnt; i++ )
>          {
> --- a/xen/include/xen/pci.h
> +++ b/xen/include/xen/pci.h
> @@ -82,6 +82,7 @@ enum {
>      DEV_TYPE_PCI,
>  };
>  
> +bool_t pci_known_segment(u16 seg);
>  int pci_device_detect(u16 seg, u8 bus, u8 dev, u8 func);
>  int scan_pci_devices(void);
>  int pdev_type(u16 seg, u8 bus, u8 devfn);



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

Kay, Allen M

2011-Oct-07 15:22 UTC

head link

[Xen-devel] RE: [PATCH] VT-d: don''t reject possibly valid DRHD or RMRR

Yes.  Ack!

Allen

-----Original Message-----
From: Jan Beulich [mailto:JBeulich@suse.com] 
Sent: Friday, October 07, 2011 12:18 AM
To: Kay, Allen M
Cc: xen-devel@lists.xensource.com
Subject: RE: [PATCH] VT-d: don''t reject possibly valid DRHD or RMRR
>>> On 07.10.11 at 03:51, "Kay, Allen M"
<allen.m.kay@intel.com> wrote:
> For RMRR case, looks like you miss the following change (or something 
> similar):
> 
> -for ( i = 0; i < rmrru->scope.devices_cnt; i++ )
> +for (; i < rmrru->scope.devices_cnt; i++ )
Indeed.
> Otherwise, the logic for handling non-zero PCI segments looks reasonable.
So do I take this as an acked-by-with-above-change?
> Allen
> 
> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com] 
> Sent: Tuesday, October 04, 2011 4:30 AM
> To: xen-devel@lists.xensource.com 
> Cc: Kay, Allen M
> Subject: [PATCH] VT-d: don''t reject possibly valid DRHD or RMRR
> 
> If a non-zero PCI segment isn''t accessible during Xen boot
(because
> firmware decided to not enter the necessary MMIO space into the E820
> table), devices referred to on those segments through DRHD or RMRR
> structures should not be rejected just because the devices can''t
be
> found.
> 
> This is in line with what is being done in at least one other case
> already: Systems with more than one PCI segment (usually high end
> ones) are assumed to have valid firmware provided data, while systems
> with just segment 0 continue to have their firmware tables validated.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> --- a/xen/drivers/passthrough/pci.c
> +++ b/xen/drivers/passthrough/pci.c
> @@ -53,6 +53,11 @@ static inline struct pci_seg *get_pseg(u
>      return radix_tree_lookup(&pci_segments, seg);
>  }
>  
> +bool_t pci_known_segment(u16 seg)
> +{
> +    return get_pseg(seg) != NULL;
> +}
> +
>  static struct pci_seg *alloc_pseg(u16 seg)
>  {
>      struct pci_seg *pseg = get_pseg(seg);
> --- a/xen/drivers/passthrough/vtd/dmar.c
> +++ b/xen/drivers/passthrough/vtd/dmar.c
> @@ -444,10 +444,14 @@ acpi_parse_one_drhd(struct acpi_dmar_ent
>      else
>      {
>          u8 b, d, f;
> -        int i, invalid_cnt = 0;
> +        unsigned int i = 0, invalid_cnt = 0;
>          void *p;
>  
> -        for ( i = 0, p = dev_scope_start; i <
dmaru->scope.devices_cnt;
> +        /* Skip checking if segment is not accessible yet. */
> +        if ( !pci_known_segment(drhd->segment) )
> +            i = UINT_MAX;
> +
> +        for ( p = dev_scope_start; i < dmaru->scope.devices_cnt;
>                i++, p += ((struct acpi_dev_scope *)p)->length )
>          {
>              if ( ((struct acpi_dev_scope *)p)->dev_type ==
ACPI_DEV_IOAPIC ||
> @@ -549,7 +553,12 @@ acpi_parse_one_rmrr(struct acpi_dmar_ent
>      else
>      {
>          u8 b, d, f;
> -        int i, ignore = 0;
> +        bool_t ignore = 0;
> +        unsigned int i = 0;
> +
> +        /* Skip checking if segment is not accessible yet. */
> +        if ( !pci_known_segment(rmrr->segment) )
> +            i = UINT_MAX;
>  
>          for ( i = 0; i < rmrru->scope.devices_cnt; i++ )
>          {
> --- a/xen/include/xen/pci.h
> +++ b/xen/include/xen/pci.h
> @@ -82,6 +82,7 @@ enum {
>      DEV_TYPE_PCI,
>  };
>  
> +bool_t pci_known_segment(u16 seg);
>  int pci_device_detect(u16 seg, u8 bus, u8 dev, u8 func);
>  int scan_pci_devices(void);
>  int pdev_type(u16 seg, u8 bus, u8 devfn);



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

Xen devel - Oct 2011 - [PATCH] VT-d: don't reject possibly valid DRHD or RMRR

[Xen-devel] [PATCH] VT-d: don''t reject possibly valid DRHD or RMRR

[Xen-devel] RE: [PATCH] VT-d: don''t reject possibly valid DRHD or RMRR

[Xen-devel] RE: [PATCH] VT-d: don''t reject possibly valid DRHD or RMRR

[Xen-devel] RE: [PATCH] VT-d: don''t reject possibly valid DRHD or RMRR