| Message ID | 945CA011AD5F084CBEA3E851C0AB28894B8AD3F2@SHSMSX101.ccr.corp.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 12.05.16 at 07:16, <quan.xu@intel.com> wrote: > Taken together, there are 3 call trees to me_wifi_quirk(): > > 1). > ...--me_wifi_quirk()--domain_context_mapping_one()--domain_context_mapping()--se > tup_hwdom_device() > > There is no use in calling this function if an earlier > error occurred. The change can be more lightweight (the detailed change is > pending). > > 2). me_wifi_quirk()--domain_context_unmap_one()--... > > As you mentioned, while in the unmap case it should > probably stay as is, to fit the "best effort" theme. > > Then I need to remove the __must_check annotation of > me_wifi_quirk(). This does not follow from the above. You again should propagate the error in all cases (unless it would overwrite an earlier error - as you're doing in various other places). Jan
On May 12, 2016 4:45 PM, Jan Beulich <JBeulich@suse.com> wrote: > >>> On 12.05.16 at 07:16, <quan.xu@intel.com> wrote: > > Taken together, there are 3 call trees to me_wifi_quirk(): > > > > 1). > > ...--me_wifi_quirk()--domain_context_mapping_one()-- > domain_context_map > > ping()--se > > tup_hwdom_device() > > > > There is no use in calling this function if an > > earlier error occurred. The change can be more lightweight (the > > detailed change is pending). > > > > 2). me_wifi_quirk()--domain_context_unmap_one()--... > > > > As you mentioned, while in the unmap case it > > should probably stay as is, to fit the "best effort" theme. > > > > Then I need to remove the __must_check annotation > > of me_wifi_quirk(). > > This does not follow from the above. You again should propagate the error in > all cases (unless it would overwrite an earlier error - as you're doing in various > other places). > Sorry, I know the item 2). is tricky, as I am confused about ' while in the unmap case it should probably stay as is, to fit the "best effort" theme '. Actually, what I need to enhance the p10 are: - drop ret - replace the if ( !seg ) - me_wifi_quirk(domain, bus, devfn, MAP_ME_PHANTOM_FUNC); + { + ret = me_wifi_quirk(domain, bus, devfn, MAP_ME_PHANTOM_FUNC); + + if ( !rc ) + rc = ret; + } ____TO_____ - if ( !seg ) - me_wifi_quirk(domain, bus, devfn, MAP_ME_PHANTOM_FUNC); + if ( !seg && !rc ) + rc = me_wifi_quirk(domain, bus, devfn, MAP_ME_PHANTOM_FUNC); btw, I found I am struggling in this v4 and I will spend more time fixing. thanks for your patience. Quan
diff --git a/xen/drivers/passthrough/vtd/extern.h b/xen/drivers/passthrough/vtd/extern.h index cbe0286..d4d37c3 100644 --- a/xen/drivers/passthrough/vtd/extern.h +++ b/xen/drivers/passthrough/vtd/extern.h @@ -91,7 +91,7 @@ int is_igd_vt_enabled_quirk(void); void platform_quirks_init(void); void vtd_ops_preamble_quirk(struct iommu* iommu); void vtd_ops_postamble_quirk(struct iommu* iommu); -void me_wifi_quirk(struct domain *domain, u8 bus, u8 devfn, int map); +int me_wifi_quirk(struct domain *domain, u8 bus, u8 devfn, int map); void pci_vtd_quirk(const struct pci_dev *); bool_t platform_supports_intremap(void); bool_t platform_supports_x2apic(void); diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c index 29cf870..0ac7894 100644 --- a/xen/drivers/passthrough/vtd/iommu.c +++ b/xen/drivers/passthrough/vtd/iommu.c @@ -1429,8 +1429,8 @@ int domain_context_mapping_one( unmap_vtd_domain_page(context_entries); - if ( !seg ) - me_wifi_quirk(domain, bus, devfn, MAP_ME_PHANTOM_FUNC); + if ( !seg && !rc ) + rc = me_wifi_quirk(domain, bus, devfn, MAP_ME_PHANTOM_FUNC); return rc; } diff --git a/xen/drivers/passthrough/vtd/quirks.c b/xen/drivers/passthrough/vtd/quirks.c index 473d1fc..3606b52 100644 --- a/xen/drivers/passthrough/vtd/quirks.c +++ b/xen/drivers/passthrough/vtd/quirks.c @@ -331,10 +331,12 @@ void __init platform_quirks_init(void) * assigning Intel integrated wifi device to a guest. */ -static void map_me_phantom_function(struct domain *domain, u32 dev, int map) +static int __must_check map_me_phantom_function(struct domain *domain, + u32 dev, int map) { struct acpi_drhd_unit *drhd; struct pci_dev *pdev; + int rc; /* find ME VT-d engine base on a real ME device */ pdev = pci_get_pdev(0, 0, PCI_DEVFN(dev, 0)); @@ -342,23 +344,27 @@ static void map_me_phantom_function(struct domain *domain, u32 dev, int map) /* map or unmap ME phantom function */ if ( map ) - domain_context_mapping_one(domain, drhd->iommu, 0, - PCI_DEVFN(dev, 7), NULL); + rc = domain_context_mapping_one(domain, drhd->iommu, 0, + PCI_DEVFN(dev, 7), NULL); else - domain_context_unmap_one(domain, drhd->iommu, 0, - PCI_DEVFN(dev, 7)); + rc = domain_context_unmap_one(domain, drhd->iommu, 0, + PCI_DEVFN(dev, 7)); + + return rc; } -void me_wifi_quirk(struct domain *domain, u8 bus, u8 devfn, int map) +int me_wifi_quirk(struct domain *domain, + u8 bus, u8 devfn, int map) { u32 id; + int rc = 0; id = pci_conf_read32(0, 0, 0, 0, 0); if ( IS_CTG(id) ) { /* quit if ME does not exist */ if ( pci_conf_read32(0, 0, 3, 0, 0) == 0xffffffff ) - return; + return 0; /* if device is WLAN device, map ME phantom device 0:3.7 */ id = pci_conf_read32(0, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), 0); @@ -372,7 +378,7 @@ void me_wifi_quirk(struct domain *domain, u8 bus, u8 devfn, int map) case 0x423b8086: case 0x423c8086: case 0x423d8086: - map_me_phantom_function(domain, 3, map); + rc = map_me_phantom_function(domain, 3, map); break; default: break; @@ -382,7 +388,7 @@ void me_wifi_quirk(struct domain *domain, u8 bus, u8 devfn, int map) { /* quit if ME does not exist */ if ( pci_conf_read32(0, 0, 22, 0, 0) == 0xffffffff ) - return; + return 0; /* if device is WLAN device, map ME phantom device 0:22.7 */ id = pci_conf_read32(0, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), 0); @@ -398,12 +404,14 @@ void me_wifi_quirk(struct domain *domain, u8 bus, u8 devfn, int map) case 0x42388086: /* Puma Peak */ case 0x422b8086: case 0x422c8086: - map_me_phantom_function(domain, 22, map); + rc = map_me_phantom_function(domain, 22, map); break; default: break; } } + + return rc; } void pci_vtd_quirk(const struct pci_dev *pdev)