Re: [Xen-devel] [RFC][v2][PATCH 00/14] Fix RMRR
On 2015/5/28 15:55, Jan Beulich wrote: On 28.05.15 at 07:48, wrote: On 2015/5/22 17:46, Jan Beulich wrote: On 22.05.15 at 11:35, wrote: As you know all devices are owned by Dom0 firstly before we create any DomU, right? Do we allow Dom0 still own a group device while assign another device in the same group? Clearly not, or - just like anything else putting the security of a system at risk - only at explicit host admin request. You're right. After we discussed internally, we're intending to cover this simply since the case of shared RMRR is a rare case according to our previous experiences. Furthermore, Xen doesn't have a good existing API to directly assign this sort of group devices and even Xen doesn't identify these devices, so currently we always assign devices one by one, right? This means we have to put more efforts to concern a good implementation to address something like, identification, atomic, hotplug and so on. Obviously, this would involve hypervisor and tools at the same time so this has a little bit of difficulty to work along with 4.6. So could we do this separately? #1. Phase 1 to 4.6 #1.1. Do a simple implementation We just prevent from that device assignment if we're assigning this sort of group devices like this, Right. @@ -2291,6 +2291,16 @@ static int intel_iommu_assign_device( PCI_BUS(bdf) == bus && PCI_DEVFN2(bdf) == devfn ) { +if ( rmrr->scope.devices_cnt > 1 ) +{ +reassign_device_ownership(d, hardware_domain, devfn, pdev); I think if this is really needed here, the check comes too late. So we can do this at the begging of this function @@ -2277,13 +2277,37 @@ static int intel_iommu_assign_device( if ( list_empty(&acpi_drhd_units) ) return -ENODEV; +seg = pdev->seg; +bus = pdev->bus; +/* + * In rare cases one given rmrr is shared by multiple devices but + * obviously this would put the security of a system at risk. So + * we should prevent from this sort of device assignment. + * + * TODO: actually we can group these devices which shared rmrr, and + * then allow all devices within a group to be assigned to same domain. + */ +for_each_rmrr_device( rmrr, bdf, i ) +{ +if ( rmrr->segment == seg && + PCI_BUS(bdf) == bus && + PCI_DEVFN2(bdf) == devfn ) +{ +if ( rmrr->scope.devices_cnt > 1 ) +{ +ret = -EPERM; +printk(XENLOG_G_ERR VTDPREFIX + " cannot assign this device with shared RMRR for Dom%d (%d)\n", + d->domain_id, ret); +return ret; +} +} +} + ret = reassign_device_ownership(hardware_domain, d, devfn, pdev); if ( ret ) return ret; -seg = pdev->seg; -bus = pdev->bus; - /* Setup rmrr identity mapping */ for_each_rmrr_device( rmrr, bdf, i ) { Thanks Tiejun ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC][v2][PATCH 00/14] Fix RMRR
>>> On 28.05.15 at 07:48, wrote: > On 2015/5/22 17:46, Jan Beulich wrote: > On 22.05.15 at 11:35, wrote: >>> As you know all devices are owned by Dom0 firstly before we create any >>> DomU, right? Do we allow Dom0 still own a group device while assign another >>> device in the same group? >> >> Clearly not, or - just like anything else putting the security of a system >> at risk - only at explicit host admin request. >> > > You're right. > > After we discussed internally, we're intending to cover this simply > since the case of shared RMRR is a rare case according to our previous > experiences. Furthermore, Xen doesn't have a good existing API to > directly assign this sort of group devices and even Xen doesn't identify > these devices, so currently we always assign devices one by one, right? > This means we have to put more efforts to concern a good implementation > to address something like, identification, atomic, hotplug and so on. > Obviously, this would involve hypervisor and tools at the same time so > this has a little bit of difficulty to work along with 4.6. > > So could we do this separately? > > #1. Phase 1 to 4.6 > > #1.1. Do a simple implementation > > We just prevent from that device assignment if we're assigning this sort > of group devices like this, Right. > @@ -2291,6 +2291,16 @@ static int intel_iommu_assign_device( >PCI_BUS(bdf) == bus && >PCI_DEVFN2(bdf) == devfn ) > { > +if ( rmrr->scope.devices_cnt > 1 ) > +{ > +reassign_device_ownership(d, hardware_domain, devfn, pdev); I think if this is really needed here, the check comes too late. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC][v2][PATCH 00/14] Fix RMRR
On 2015/5/22 17:46, Jan Beulich wrote: On 22.05.15 at 11:35, wrote: As you know all devices are owned by Dom0 firstly before we create any DomU, right? Do we allow Dom0 still own a group device while assign another device in the same group? Clearly not, or - just like anything else putting the security of a system at risk - only at explicit host admin request. You're right. After we discussed internally, we're intending to cover this simply since the case of shared RMRR is a rare case according to our previous experiences. Furthermore, Xen doesn't have a good existing API to directly assign this sort of group devices and even Xen doesn't identify these devices, so currently we always assign devices one by one, right? This means we have to put more efforts to concern a good implementation to address something like, identification, atomic, hotplug and so on. Obviously, this would involve hypervisor and tools at the same time so this has a little bit of difficulty to work along with 4.6. So could we do this separately? #1. Phase 1 to 4.6 #1.1. Do a simple implementation We just prevent from that device assignment if we're assigning this sort of group devices like this, @@ -2291,6 +2291,16 @@ static int intel_iommu_assign_device( PCI_BUS(bdf) == bus && PCI_DEVFN2(bdf) == devfn ) { +if ( rmrr->scope.devices_cnt > 1 ) +{ +reassign_device_ownership(d, hardware_domain, devfn, pdev); +printk(XENLOG_G_ERR VTDPREFIX + " cannot assign any device with RMRR for Dom%d (%d)\n", + rmrr->base_address, rmrr->end_address, + d->domain_id, ret); +ret = -EPERM; +break; +} ret = rmrr_identity_mapping(d, 1, rmrr, flag); if ( ret ) { Note this is just one draft code to show our idea. And I'm also concerning if we need to introduce a flag to bypass this to make sure we still have a approach to our original behavior. #1.2. Post a design We'd like to post a preliminary design to Xen community to get a better solution. #2. Phase 2 after 4.6 Once the design is clear we will start writing patches to address this completely. So any idea? Thanks Tiejun ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC][v2][PATCH 00/14] Fix RMRR
>>> On 22.05.15 at 11:35, wrote: > As you know all devices are owned by Dom0 firstly before we create any > DomU, right? Do we allow Dom0 still own a group device while assign another > device in the same group? Clearly not, or - just like anything else putting the security of a system at risk - only at explicit host admin request. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [RFC][v2][PATCH 00/14] Fix RMRR
v2: * Instead of that fixed predefined rdm memory boundary, we'd like to introduce a parameter, "rdm_mem_boundary", to set this threshold value. * Remove that existing USB hack. * Make sure the MMIO regions all fit in the available resource window * Rename our policy, "force/try" -> "strict/relaxed" * Indeed, Wei and Jan gave me more and more comments to refine codes * Code style * Better and reasonable code implementation * Correct or improve code comments. * A little bit to work well with ARM. Open: * We should fail assigning device which has a shared RMRR with another device. We can only do group assignment when RMRR is shared among devices. We need more time to figure a good policy/way out because something is not clear to me. As you know all devices are owned by Dom0 firstly before we create any DomU, right? Do we allow Dom0 still own a group device while assign another device in the same group? Really appreciate any comments to policy. v1: RMRR is an acronym for Reserved Memory Region Reporting, expected to be used for legacy usages (such as USB, UMA Graphics, etc.) requiring reserved memory. Special treatment is required in system software to setup those reserved regions in IOMMU translation structures, otherwise passing through a device with RMRR reported may not work correctly. This patch set tries to enhance existing Xen RMRR implementation to fix various reported and theoretical problems. Most noteworthy changes are to setup identity mapping in p2m layer and handle possible conflicts between reported regions and gfn space. Initial proposal can be found at: http://lists.xenproject.org/archives/html/xen-devel/2015-01/msg00524.html and after a long discussion a summarized agreement is here: http://lists.xen.org/archives/html/xen-devel/2015-01/msg01580.html Below is a key summary of this patch set according to agreed proposal: 1. Use RDM (Reserved Device Memory) name in user space as a general description instead of using ACPI RMRR name directly. 2. Introduce configuration parameters to allow user control both per-device and global RDM resources along with desired policies upon a detected conflict. 3. Introduce a new hypercall to query global and per-device RDM resources. 4. Extend libxl to be a central place to manage RDM resources and handle potential conflicts between reserved regions and gfn space. One simplification goal is made to keep existing lowmem / mmio / highmem layout which is passed around various function blocks. So a reasonable assumption is made, that conflicts falling into below areas are not re-arranged otherwise it will result in a more scattered layout: a) in highmem region (>4G) b) in lowmem region, and below a predefined boundary (default 2G) a) is a new assumption not discussed before. From VT-d spec this is possible but no such observation in real-world. So we can make this reasonable assumption until there's real usage on it. 5. Extend XENMEM_set_memory_map usable for HVM guest, and then have libxl to use that hypercall to carry RDM information to hvmloader. There is one difference from original discussion. Previously we discussed to introduce a new E820 type specifically for RDM entries. After more thought we think it's OK to just tag them as E820_reserved. Actually hvmloader doesn't need to know whether the reserved entries come from RDM or from other purposes. 6. Then in hvmloader the change is generic for XENMEM_memory_map change. Given a predefined memory layout, hvmloader should avoid allocating all reserved entries for other usages (opregion, mmio, etc.) 7. Extend existing device passthrough hypercall to carry conflict handling policy. 8. Setup identity map in p2m layer for RMRRs reported for the given device. And conflicts are handled according to specified policy in hypercall. Current patch set contains core enhancements calling for comments. There are still several tasks not implemented now. We'll include them in final version after RFC is agreed: - remove existing USB hack - detect and fail assigning device which has a shared RMRR with another device - add a config parameter to configure that memory boundary flexibly - In the case of hotplug we also need to figure out a way to fix that policy conflict between the per-pci policy and the global policy but firstly we think we'd better collect some good or correct ideas to step next in RFC. So here I made this as RFC to collect your any comments. Jan Beulich (1): introduce XENMEM_reserved_device_memory_map Tiejun Chen (13): tools: introduce some new parameters to set rdm policy tools/libxc: Expose new hypercall xc_reserved_device_memory_map tools/libxl: detect and avoid conflicts with RDM xen/x86/p2m: introduce set_identity_p2m_entry xen:vtd: create RMRR mapping xen/passthrough: extend hypercall to support rdm reservation policy tools: exten