When calculating use_iommu, we wanted to first detect whether DMAR is
enabled, then check whether PT is enabled if DMAR is enabled.  However
in the current code we used "&" rather than "&&" so the ordering
requirement is lost (instead it'll be an "AND" operation).  This could
introduce errors dumped in QEMU console when rebooting a guest with
both assigned device and vIOMMU, like:

  qemu-system-x86_64: vtd_dev_to_context_entry: invalid root entry:
  rsvd=0xf000ff53f000e2c3, val=0xf000ff53f000ff53 (reserved nonzero)

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 hw/i386/intel_iommu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 8b72735650..6d5cc1d039 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -1153,7 +1153,7 @@ static bool vtd_switch_address_space(VTDAddressSpace *as)
 
     assert(as);
 
-    use_iommu = as->iommu_state->dmar_enabled & !vtd_dev_pt_enabled(as);
+    use_iommu = as->iommu_state->dmar_enabled && !vtd_dev_pt_enabled(as);
 
     trace_vtd_switch_address_space(pci_bus_num(as->bus),
                                    VTD_PCI_SLOT(as->devfn),
-- 
2.17.1

On 2019/1/8 下午7:47, Peter Xu wrote:
> When calculating use_iommu, we wanted to first detect whether DMAR is
> enabled, then check whether PT is enabled if DMAR is enabled.  However
> in the current code we used "&" rather than "&&" so the ordering
> requirement is lost (instead it'll be an "AND" operation).  This could
> introduce errors dumped in QEMU console when rebooting a guest with
> both assigned device and vIOMMU, like:
>
>    qemu-system-x86_64: vtd_dev_to_context_entry: invalid root entry:
>    rsvd=0xf000ff53f000e2c3, val=0xf000ff53f000ff53 (reserved nonzero)
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>   hw/i386/intel_iommu.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 8b72735650..6d5cc1d039 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -1153,7 +1153,7 @@ static bool vtd_switch_address_space(VTDAddressSpace *as)
>   
>       assert(as);
>   
> -    use_iommu = as->iommu_state->dmar_enabled & !vtd_dev_pt_enabled(as);
> +    use_iommu = as->iommu_state->dmar_enabled && !vtd_dev_pt_enabled(as);
>   
>       trace_vtd_switch_address_space(pci_bus_num(as->bus),
>                                      VTD_PCI_SLOT(as->devfn),


Acked-by: Jason Wang <jasowang@redhat.com>