[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [v3][PATCH 04/16] xen/passthrough: extend hypercall to support rdm reservation policy
> From: Chen, Tiejun > Sent: Thursday, June 11, 2015 9:15 AM > > This patch extends the existing hypercall to support rdm reservation policy. > We return error or just throw out a warning message depending on whether > the policy is "strict" or "relaxed" when reserving RDM regions in pfn space. > Note in some special cases, e.g. add a device to hwdomain, and remove a > device from user domain, 'relaxed' is fine enough since this is always safe > to hwdomain. could you elaborate " add a device to hwdomain, and remove a device from user domain "? move a device from user domain to hwdomain or completely irrelevant? > > Signed-off-by: Tiejun Chen <tiejun.chen@xxxxxxxxx> > --- > xen/arch/x86/mm/p2m.c | 8 +++++++- > xen/drivers/passthrough/amd/pci_amd_iommu.c | 3 ++- > xen/drivers/passthrough/arm/smmu.c | 2 +- > xen/drivers/passthrough/device_tree.c | 11 ++++++++++- > xen/drivers/passthrough/pci.c | 10 ++++++---- > xen/drivers/passthrough/vtd/iommu.c | 20 ++++++++++++-------- > xen/include/asm-x86/p2m.h | 2 +- > xen/include/public/domctl.h | 5 +++++ > xen/include/xen/iommu.h | 2 +- > 9 files changed, 45 insertions(+), 18 deletions(-) > > diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c > index c7198a5..3fcdcac 100644 > --- a/xen/arch/x86/mm/p2m.c > +++ b/xen/arch/x86/mm/p2m.c > @@ -899,7 +899,7 @@ int set_mmio_p2m_entry(struct domain *d, unsigned long > gfn, > mfn_t mfn, > } > > int set_identity_p2m_entry(struct domain *d, unsigned long gfn, > - p2m_access_t p2ma) > + p2m_access_t p2ma, u32 flag) > { > p2m_type_t p2mt; > p2m_access_t a; > @@ -924,6 +924,12 @@ int set_identity_p2m_entry(struct domain *d, unsigned > long gfn, > printk(XENLOG_G_WARNING > "Cannot identity map d%d:%lx, already mapped to %lx.\n", > d->domain_id, gfn, mfn_x(mfn)); > + > + if ( flag == XEN_DOMCTL_DEV_RDM_RELAXED ) > + { > + ret = 0; > + printk(XENLOG_G_WARNING "Some devices may work failed.\n"); Do you need this extra printk? The warning message is already given several lines above and here you just need to change return value for relaxed policy. > + } > } > > gfn_unlock(p2m, gfn, 0); > diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c > b/xen/drivers/passthrough/amd/pci_amd_iommu.c > index e83bb35..920b35a 100644 > --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c > +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c > @@ -394,7 +394,8 @@ static int reassign_device(struct domain *source, struct > domain > *target, > } > > static int amd_iommu_assign_device(struct domain *d, u8 devfn, > - struct pci_dev *pdev) > + struct pci_dev *pdev, > + u32 flag) > { > struct ivrs_mappings *ivrs_mappings = get_ivrs_mappings(pdev->seg); > int bdf = PCI_BDF2(pdev->bus, devfn); > diff --git a/xen/drivers/passthrough/arm/smmu.c > b/xen/drivers/passthrough/arm/smmu.c > index 6cc4394..9a667e9 100644 > --- a/xen/drivers/passthrough/arm/smmu.c > +++ b/xen/drivers/passthrough/arm/smmu.c > @@ -2605,7 +2605,7 @@ static void arm_smmu_destroy_iommu_domain(struct > iommu_domain *domain) > } > > static int arm_smmu_assign_dev(struct domain *d, u8 devfn, > - struct device *dev) > + struct device *dev, u32 flag) > { > struct iommu_domain *domain; > struct arm_smmu_xen_domain *xen_domain; > diff --git a/xen/drivers/passthrough/device_tree.c > b/xen/drivers/passthrough/device_tree.c > index 5d3842a..ea85645 100644 > --- a/xen/drivers/passthrough/device_tree.c > +++ b/xen/drivers/passthrough/device_tree.c > @@ -52,7 +52,8 @@ int iommu_assign_dt_device(struct domain *d, struct > dt_device_node *dev) > goto fail; > } > > - rc = hd->platform_ops->assign_device(d, 0, dt_to_dev(dev)); > + rc = hd->platform_ops->assign_device(d, 0, dt_to_dev(dev), > + XEN_DOMCTL_DEV_NO_RDM); > > if ( rc ) > goto fail; > @@ -148,6 +149,14 @@ int iommu_do_dt_domctl(struct xen_domctl *domctl, struct > domain *d, > if ( domctl->u.assign_device.dev != XEN_DOMCTL_DEV_DT ) > break; > > + if ( domctl->u.assign_device.flag == XEN_DOMCTL_DEV_NO_RDM ) > + { > + printk(XENLOG_G_ERR "XEN_DOMCTL_assign_device: assign \"%s\"" > + " to dom%u failed (%d) since we don't support RDM.\n", > + dt_node_full_name(dev), d->domain_id, ret); > + break; > + } > + > if ( unlikely(d->is_dying) ) > { > ret = -EINVAL; > diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c > index e30be43..557c87e 100644 > --- a/xen/drivers/passthrough/pci.c > +++ b/xen/drivers/passthrough/pci.c > @@ -1335,7 +1335,7 @@ static int device_assigned(u16 seg, u8 bus, u8 devfn) > return pdev ? 0 : -EBUSY; > } > > -static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn) > +static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 > flag) > { > struct hvm_iommu *hd = domain_hvm_iommu(d); > struct pci_dev *pdev; > @@ -1371,7 +1371,7 @@ static int assign_device(struct domain *d, u16 seg, u8 > bus, u8 > devfn) > > pdev->fault.count = 0; > > - if ( (rc = hd->platform_ops->assign_device(d, devfn, pci_to_dev(pdev))) ) > + if ( (rc = hd->platform_ops->assign_device(d, devfn, pci_to_dev(pdev), > flag)) ) > goto done; > > for ( ; pdev->phantom_stride; rc = 0 ) > @@ -1379,7 +1379,7 @@ static int assign_device(struct domain *d, u16 seg, u8 > bus, u8 > devfn) > devfn += pdev->phantom_stride; > if ( PCI_SLOT(devfn) != PCI_SLOT(pdev->devfn) ) > break; > - rc = hd->platform_ops->assign_device(d, devfn, pci_to_dev(pdev)); > + rc = hd->platform_ops->assign_device(d, devfn, pci_to_dev(pdev), > flag); > if ( rc ) > printk(XENLOG_G_WARNING "d%d: assign %04x:%02x:%02x.%u failed > (%d)\n", > d->domain_id, seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), > @@ -1496,6 +1496,7 @@ int iommu_do_pci_domctl( > { > u16 seg; > u8 bus, devfn; > + u32 flag; > int ret = 0; > uint32_t machine_sbdf; > > @@ -1577,9 +1578,10 @@ int iommu_do_pci_domctl( > seg = machine_sbdf >> 16; > bus = PCI_BUS(machine_sbdf); > devfn = PCI_DEVFN2(machine_sbdf); > + flag = domctl->u.assign_device.flag; > > ret = device_assigned(seg, bus, devfn) ?: > - assign_device(d, seg, bus, devfn); > + assign_device(d, seg, bus, devfn, flag); > if ( ret == -ERESTART ) > ret = hypercall_create_continuation(__HYPERVISOR_domctl, > "h", u_domctl); > diff --git a/xen/drivers/passthrough/vtd/iommu.c > b/xen/drivers/passthrough/vtd/iommu.c > index 31ce1af..d7c9e1c 100644 > --- a/xen/drivers/passthrough/vtd/iommu.c > +++ b/xen/drivers/passthrough/vtd/iommu.c > @@ -1808,7 +1808,8 @@ static void iommu_set_pgd(struct domain *d) > } > > static int rmrr_identity_mapping(struct domain *d, bool_t map, > - const struct acpi_rmrr_unit *rmrr) > + const struct acpi_rmrr_unit *rmrr, > + u32 flag) > { > unsigned long base_pfn = rmrr->base_address >> PAGE_SHIFT_4K; > unsigned long end_pfn = PAGE_ALIGN_4K(rmrr->end_address) >> > PAGE_SHIFT_4K; > @@ -1856,7 +1857,7 @@ static int rmrr_identity_mapping(struct domain *d, > bool_t map, > > while ( base_pfn < end_pfn ) > { > - int err = set_identity_p2m_entry(d, base_pfn, p2m_access_rw); > + int err = set_identity_p2m_entry(d, base_pfn, p2m_access_rw, flag); > > if ( err ) > return err; > @@ -1899,7 +1900,8 @@ static int intel_iommu_add_device(u8 devfn, struct > pci_dev > *pdev) > PCI_BUS(bdf) == pdev->bus && > PCI_DEVFN2(bdf) == devfn ) > { > - ret = rmrr_identity_mapping(pdev->domain, 1, rmrr); > + ret = rmrr_identity_mapping(pdev->domain, 1, rmrr, > + XEN_DOMCTL_DEV_RDM_RELAXED); Why did you hardcode relax policy here? Shouldn't the policy come from hypercall flag? > if ( ret ) > dprintk(XENLOG_ERR VTDPREFIX, "d%d: RMRR mapping failed\n", > pdev->domain->domain_id); > @@ -1940,7 +1942,8 @@ static int intel_iommu_remove_device(u8 devfn, struct > pci_dev > *pdev) > PCI_DEVFN2(bdf) != devfn ) > continue; > > - rmrr_identity_mapping(pdev->domain, 0, rmrr); > + rmrr_identity_mapping(pdev->domain, 0, rmrr, > + XEN_DOMCTL_DEV_RDM_RELAXED); ditto > } > > return domain_context_unmap(pdev->domain, devfn, pdev); > @@ -2098,7 +2101,7 @@ static void __hwdom_init setup_hwdom_rmrr(struct domain > *d) > spin_lock(&pcidevs_lock); > for_each_rmrr_device ( rmrr, bdf, i ) > { > - ret = rmrr_identity_mapping(d, 1, rmrr); > + ret = rmrr_identity_mapping(d, 1, rmrr, XEN_DOMCTL_DEV_RDM_RELAXED); > if ( ret ) > dprintk(XENLOG_ERR VTDPREFIX, > "IOMMU: mapping reserved region failed\n"); > @@ -2241,7 +2244,8 @@ static int reassign_device_ownership( > PCI_BUS(bdf) == pdev->bus && > PCI_DEVFN2(bdf) == devfn ) > { > - ret = rmrr_identity_mapping(source, 0, rmrr); > + ret = rmrr_identity_mapping(source, 0, rmrr, > + XEN_DOMCTL_DEV_RDM_RELAXED); > if ( ret != -ENOENT ) > return ret; > } > @@ -2265,7 +2269,7 @@ static int reassign_device_ownership( > } > > static int intel_iommu_assign_device( > - struct domain *d, u8 devfn, struct pci_dev *pdev) > + struct domain *d, u8 devfn, struct pci_dev *pdev, u32 flag) > { > struct acpi_rmrr_unit *rmrr; > int ret = 0, i; > @@ -2294,7 +2298,7 @@ static int intel_iommu_assign_device( > PCI_BUS(bdf) == bus && > PCI_DEVFN2(bdf) == devfn ) > { > - ret = rmrr_identity_mapping(d, 1, rmrr); > + ret = rmrr_identity_mapping(d, 1, rmrr, flag); > if ( ret ) > { > reassign_device_ownership(d, hardware_domain, devfn, pdev); > diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h > index 95b6266..a80b4f8 100644 > --- a/xen/include/asm-x86/p2m.h > +++ b/xen/include/asm-x86/p2m.h > @@ -545,7 +545,7 @@ int clear_mmio_p2m_entry(struct domain *d, unsigned long > gfn, > mfn_t mfn); > > /* Set identity addresses in the p2m table (for pass-through) */ > int set_identity_p2m_entry(struct domain *d, unsigned long gfn, > - p2m_access_t p2ma); > + p2m_access_t p2ma, u32 flag); > > /* Add foreign mapping to the guest's p2m table. */ > int p2m_add_foreign(struct domain *tdom, unsigned long fgfn, > diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h > index bc45ea5..2f9e40e 100644 > --- a/xen/include/public/domctl.h > +++ b/xen/include/public/domctl.h > @@ -478,6 +478,11 @@ struct xen_domctl_assign_device { > XEN_GUEST_HANDLE_64(char) path; /* path to the device tree node > */ > } dt; > } u; > + /* IN */ > +#define XEN_DOMCTL_DEV_NO_RDM 0 > +#define XEN_DOMCTL_DEV_RDM_RELAXED 1 > +#define XEN_DOMCTL_DEV_RDM_STRICT 2 I don't understand why we require a NO_RDM flag. Whether there is RDM associated with a given device, it's reported by system BIOS or through cmdline extension in coming patch. Why do we require the hypercall to ask NO_RDM to hypervisor? The only flags we want to pass to hypervisor is relaxed/strict policy, so hypervisor know whether to fail or warn upon caught conflicts of identity mapping... > + uint32_t flag; /* flag of assigned device */ > }; > typedef struct xen_domctl_assign_device xen_domctl_assign_device_t; > DEFINE_XEN_GUEST_HANDLE(xen_domctl_assign_device_t); > diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h > index e2f584d..02b2b02 100644 > --- a/xen/include/xen/iommu.h > +++ b/xen/include/xen/iommu.h > @@ -140,7 +140,7 @@ struct iommu_ops { > int (*add_device)(u8 devfn, device_t *dev); > int (*enable_device)(device_t *dev); > int (*remove_device)(u8 devfn, device_t *dev); > - int (*assign_device)(struct domain *, u8 devfn, device_t *dev); > + int (*assign_device)(struct domain *, u8 devfn, device_t *dev, u32 flag); > int (*reassign_device)(struct domain *s, struct domain *t, > u8 devfn, device_t *dev); > #ifdef HAS_PCI > -- > 1.9.1 _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |