Re: [PATCH 1/2] VT-d: apply quirks at device setup time rather than only at boot

[Andrew Cooper <andrew.cooper3@citrix.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3110 bytes --]

On 28/04/14 09:01, Jan Beulich wrote:
> Accessing extended config space may not be possible at boot time, e.g.
> when the memory space used by MMCFG is reserved only via ACPI tables,
> but not in the E820/UEFI memory maps (which we need Dom0 to tell us
> about). Consequently the change here still leaves the issue unaddressed
> for systems where the extended config space remains inaccessible (due
> to firmware bugs, i.e. not properly reserving the address space of
> those regions).
>
> With the respective messages now potentially getting logged more than
> once, we ought to consider whether we should issue them only if we in
> fact were required to do any masking (i.e. if the relevant mask bits
> weren't already set).
>
> This is CVE-2013-3495 / XSA-59.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

I would agree that log messages should only be presented if Xen had to
change system state.

Content-wise, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

>
> --- a/xen/drivers/passthrough/vtd/extern.h
> +++ b/xen/drivers/passthrough/vtd/extern.h
> @@ -99,7 +99,7 @@ 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);
> -void pci_vtd_quirk(struct pci_dev *pdev);
> +void pci_vtd_quirk(const struct pci_dev *);
>  int platform_supports_intremap(void);
>  int platform_supports_x2apic(void);
>  
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -1483,6 +1483,9 @@ static int domain_context_mapping(
>          break;
>      }
>  
> +    if ( !ret && devfn == pdev->devfn )
> +        pci_vtd_quirk(pdev);
> +
>      return ret;
>  }
>  
> @@ -1922,6 +1925,8 @@ static int intel_iommu_enable_device(str
>      struct acpi_drhd_unit *drhd = acpi_find_matched_drhd_unit(pdev);
>      int ret = drhd ? ats_device(pdev, drhd) : -ENODEV;
>  
> +    pci_vtd_quirk(pdev);
> +
>      if ( ret <= 0 )
>          return ret;
>  
> @@ -1994,12 +1999,7 @@ static int intel_iommu_remove_device(u8 
>  
>  static int __hwdom_init setup_hwdom_device(u8 devfn, struct pci_dev *pdev)
>  {
> -    int err;
> -
> -    err = domain_context_mapping(pdev->domain, devfn, pdev);
> -    if ( !err && devfn == pdev->devfn )
> -        pci_vtd_quirk(pdev);
> -    return err;
> +    return domain_context_mapping(pdev->domain, devfn, pdev);
>  }
>  
>  void clear_fault_bits(struct iommu *iommu)
> --- a/xen/drivers/passthrough/vtd/quirks.c
> +++ b/xen/drivers/passthrough/vtd/quirks.c
> @@ -385,7 +385,7 @@ void me_wifi_quirk(struct domain *domain
>   *   - This can cause system failure upon non-fatal VT-d faults
>   *   - Potential security issue if malicious guest trigger VT-d faults
>   */
> -void __hwdom_init pci_vtd_quirk(struct pci_dev *pdev)
> +void pci_vtd_quirk(const struct pci_dev *pdev)
>  {
>      int seg = pdev->seg;
>      int bus = pdev->bus;
>
>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel


[-- Attachment #1.2: Type: text/html, Size: 3949 bytes --]

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

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