Re: [Xen-devel] Xen-unstable: AMD-Vi: update_paging_mode Try to access pdev_list without aquiring pcidevs_lock.

[Sander Eikelenboom <linux@xxxxxxxxxxxxxx>]

On 12/11/2019 12:05, Jan Beulich wrote:
> On 11.11.2019 22:38, Sander Eikelenboom wrote:
>> When supplying "pci=nomsi" to the guest kernel, the device works fine,
>> and I don't get the "INVALID_DEV_REQUEST".
>>
>> After reverting 1b00c16bdf, the device works fine 
>> and I don't get the INVALID_DEV_REQUEST, 
> 
> Could you give the patch below a try? That commit took care of only
> securing ourselves, but not of relaxing things again when a device
> gets handed to a guest for actual use.
> 
> Jan

Hi Jan,

CC'ed Juergen, as he seems to be dropped off the CC-list at some time.

Just tested this patch: 
the device works fine and I don't get the INVALID_DEV_REQUEST.

This was the last remaining issue around pci passthrough I encountered, 
with all patches applied (yours and Anthony's) pci passthrough for me 
seems to work again as I was used to.

Thanks again for fixing the issues and providing the right educated guesses!

--
Sander


> AMD/IOMMU: restore DTE fields in amd_iommu_setup_domain_device()
> 
> Commit 1b00c16bdf ("AMD/IOMMU: pre-fill all DTEs right after table
> allocation") moved ourselves into a more secure default state, but
> didn't take sufficient care to also undo the effects when handing a
> previously disabled device back to a(nother) domain. Put the fields
> that may have been changed elsewhere back to their intended values
> (some fields amd_iommu_disable_domain_device() touches don't
> currently get written anywhere else, and hence don't need modifying
> here).
> 
> Reported-by: Sander Eikelenboom <linux@xxxxxxxxxxxxxx>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> --- unstable.orig/xen/drivers/passthrough/amd/pci_amd_iommu.c
> +++ unstable/xen/drivers/passthrough/amd/pci_amd_iommu.c
> @@ -114,11 +114,21 @@ static void amd_iommu_setup_domain_devic
>  
>      if ( !dte->v || !dte->tv )
>      {
> +        const struct ivrs_mappings *ivrs_dev;
> +
>          /* bind DTE to domain page-tables */
>          amd_iommu_set_root_page_table(
>              dte, page_to_maddr(hd->arch.root_table), domain->domain_id,
>              hd->arch.paging_mode, valid);
>  
> +        /* Undo what amd_iommu_disable_domain_device() may have done. */
> +        ivrs_dev = &get_ivrs_mappings(iommu->seg)[req_id];
> +        if ( dte->it_root )
> +            dte->int_ctl = IOMMU_DEV_TABLE_INT_CONTROL_TRANSLATED;
> +        dte->iv = iommu_intremap;
> +        dte->ex = ivrs_dev->dte_allow_exclusion;
> +        dte->sys_mgt = MASK_EXTR(ivrs_dev->device_flags, 
> ACPI_IVHD_SYSTEM_MGMT);
> +
>          if ( pci_ats_device(iommu->seg, bus, pdev->devfn) &&
>               iommu_has_cap(iommu, PCI_CAP_IOTLB_SHIFT) )
>              dte->i = ats_enabled;
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel