From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Chen, Tiejun" Subject: Re: [RFC][v2][PATCH 00/14] Fix RMRR Date: Fri, 29 May 2015 15:58:26 +0800 Message-ID: <55681C22.8060804@intel.com> References: <1432287314-4388-1-git-send-email-tiejun.chen@intel.com> <555F172A020000780007D2B4@mail.emea.novell.com> <5566AC4B.1000701@intel.com> <5566E601020000780007E632@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <5566E601020000780007E632@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: tim@xen.org, kevin.tian@intel.com, wei.liu2@citrix.com, ian.campbell@citrix.com, andrew.cooper3@citrix.com, Ian.Jackson@eu.citrix.com, xen-devel@lists.xen.org, stefano.stabellini@citrix.com, yang.z.zhang@intel.com List-Id: xen-devel@lists.xenproject.org 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