From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Chen, Tiejun" Subject: Re: [v5][PATCH 03/10] xen:x86: define a new hypercall to get RMRR mappings Date: Wed, 03 Sep 2014 16:31:29 +0800 Message-ID: <5406D1E1.3050601@intel.com> References: <1409050980-21933-1-git-send-email-tiejun.chen@intel.com> <1409050980-21933-4-git-send-email-tiejun.chen@intel.com> <53FC774D.8000501@citrix.com> <53FC9BB1020000780002D986@mail.emea.novell.com> <53FD3669.3070705@intel.com> <53FD9C03020000780002DEFF@mail.emea.novell.com> <53FD86E9.4020205@intel.com> <53FE92C6.6050605@intel.com> <53FEED4F020000780002E76C@mail.emea.novell.com> <53FED5AF.7080803@intel.com> <53FED802.7080707@intel.com> <53FEF9F8020000780002E7E2@mail.emea.novell.com> <53FFED5D.7040406@intel.com> <54006196020000780002EFC6@mail.emea.novell.com> <54043FEA.6080100@intel.com> <540466A8020000780002F656@mail.emea.novell.com> <540594F2.4000406@intel.com> <5405B4E1020000780002FCBC@mail.emea.novell.com> <5405A597.9040103@intel.com> <5405DF00020000780002FD52@mail.emea.novell.com> <540672A7.9050000@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <540672A7.9050000@intel.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: kevin.tian@intel.com, ian.campbell@citrix.com, stefano.stabellini@eu.citrix.com, Andrew Cooper , ian.jackson@eu.citrix.com, xen-devel@lists.xen.org, yang.z.zhang@intel.com List-Id: xen-devel@lists.xenproject.org On 2014/9/3 9:45, Chen, Tiejun wrote: > On 2014/9/2 21:15, Jan Beulich wrote: >>>>> On 02.09.14 at 13:10, wrote: >>> On 2014/9/2 18:15, Jan Beulich wrote: >>>>>>> On 02.09.14 at 11:59, wrote: >>>>> + case XENMEM_reserved_device_memory_map: >>>>> + { >>>>> + struct xen_mem_reserved_device_memory *xmrdm = NULL; >>>>> + struct xen_mem_reserved_device_memory_map xmrdmm; >>>>> + XEN_GUEST_HANDLE(xen_mem_reserved_device_memory_t) buffer; >>>>> + XEN_GUEST_HANDLE_PARAM(xen_mem_reserved_device_memory_t) >>>>> buffer_param; >>>>> + const struct iommu_ops *ops = iommu_get_ops(); >>>>> + unsigned int nr_entries = 0; >>>>> + unsigned int i = 0; >>>>> + >>>>> + xmrdm = ops->get_device_reserved_memory(&nr_entries); >>> >>> Do we still need this iommu_ops somewhere else? >> >> Not this one, but another one (as I had described further down). >> >>>>> + if ( !nr_entries ) >>> >>> Do we still need this 'nr_entries' here? >> >> Doesn't look like so. But it's you coding this up - you ought to clearly >> see what is needed and what not. >> > > I mean we need to get 'nr_entries' before any real operations since if > that is zero, any following operations are pointless. And even that is > nonzero, but actually the user don't know how many buffer should be > passed, so often we may need to notify user again at the first time then > the user call this again with a appropriate buffer. Right? > > So how do we get this 'nr_entries'? Seems we have to to go VT-D stuff to > walk that list or other similar approaches. If so, why we still get > those real mapping entries later by accessing VT-D stuff again? > > Shouldn't we figure out a approach we just call one time? > I update this patch based on this point: xen/x86: define a hypercall to expose device reserved memory maps We need such a new hypercall to get all device reserved memory maps, then we can check if these maps are overlapping MMIO/RAM. Note currently just address RMRR issue. Signed-off-by: Tiejun Chen --- xen/arch/x86/mm.c | 23 +++++++++++++++++++++ xen/drivers/passthrough/vtd/dmar.c | 41 ++++++++++++++++++++++++++++++++++++++ xen/include/public/memory.h | 37 +++++++++++++++++++++++++++++++++- xen/include/xen/mm.h | 3 +++ 4 files changed, 103 insertions(+), 1 deletion(-) diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c index d23cb3f..db3345b 100644 --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -180,6 +180,14 @@ static uint32_t base_disallow_mask; is_pv_domain(d)) ? \ L1_DISALLOW_MASK : (L1_DISALLOW_MASK & ~PAGE_CACHE_ATTRS)) +extern int +do_copy_device_reserved_memory(struct xen_mem_reserved_device_memory_map *xmrdmmap); +static int +copy_drmmap_to_guest(drmmap_callback_t f, struct xen_mem_reserved_device_memory_map *xmrdmmap) +{ + return f(xmrdmmap); +} + static void __init init_frametable_chunk(void *start, void *end) { unsigned long s = (unsigned long)start; @@ -4842,6 +4850,21 @@ long arch_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg) return rc; } + case XENMEM_reserved_device_memory_map: + { + struct xen_mem_reserved_device_memory_map xmrdmmap; + + if ( copy_from_guest(&xmrdmmap, arg, 1) ) + return -EFAULT; + + rc = copy_drmmap_to_guest(do_copy_device_reserved_memory, &xmrdmmap); + + if ( __copy_to_guest(arg, &xmrdmmap, 1) ) + rc = -EFAULT; + + return rc; + } + default: return subarch_memory_op(cmd, arg); } diff --git a/xen/drivers/passthrough/vtd/dmar.c b/xen/drivers/passthrough/vtd/dmar.c index 1152c3a..6bc5def 100644 --- a/xen/drivers/passthrough/vtd/dmar.c +++ b/xen/drivers/passthrough/vtd/dmar.c @@ -28,6 +28,7 @@ #include #include #include +#include #include #include "dmar.h" #include "iommu.h" @@ -681,6 +682,46 @@ acpi_parse_one_rmrr(struct acpi_dmar_header *header) return ret; } +int +do_copy_device_reserved_memory(struct xen_mem_reserved_device_memory_map *xmrdmmap) +{ + struct acpi_rmrr_unit *rmrru; + struct xen_mem_reserved_device_memory xmrdm; + int i = 0, rc = 0; + XEN_GUEST_HANDLE_PARAM(xen_mem_reserved_device_memory_t) buffer_param; + XEN_GUEST_HANDLE(xen_mem_reserved_device_memory_t) buffer; + + if ( list_empty(&acpi_rmrr_units) ) + return -ENOENT; + + buffer_param = guest_handle_cast(xmrdmmap->buffer, + xen_mem_reserved_device_memory_t); + buffer = guest_handle_from_param(buffer_param, + xen_mem_reserved_device_memory_t); + if ( !guest_handle_okay(buffer, xmrdmmap->nr_entries) ) + return -EFAULT; + + list_for_each_entry(rmrru, &acpi_rmrr_units, list) + { + if ( i < xmrdmmap->nr_entries ) + { + xmrdm.start_pfn = rmrru->base_address >> PAGE_SHIFT; + xmrdm.nr_pages = PAGE_ALIGN(rmrru->end_address - + rmrru->base_address) / + PAGE_SIZE; + if ( __copy_to_guest_offset(buffer, i, &xmrdm, 1) ) + return -EFAULT; + } + else + rc = -ENOBUFS; + i++; + } + + xmrdmmap->nr_entries = i; + + return rc; +} + static int __init acpi_parse_one_atsr(struct acpi_dmar_header *header) { diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h index 2c57aa0..628c99c 100644 --- a/xen/include/public/memory.h +++ b/xen/include/public/memory.h @@ -523,7 +523,42 @@ DEFINE_XEN_GUEST_HANDLE(xen_mem_sharing_op_t); #endif /* defined(__XEN__) || defined(__XEN_TOOLS__) */ -/* Next available subop number is 26 */ +/* + * For legacy reasons, some devices must be configured with special memory + * regions to function correctly. The guest must avoid using any of these + * regions. + * + * - Reserved memory Region Reporting Structure, + * So returns the device reserved memory map as it was when the domain + * was started. + */ +#define XENMEM_reserved_device_memory_map 26 +struct xen_mem_reserved_device_memory { + /* Start PFN of the current mapping of the page. */ + xen_pfn_t start_pfn; + /* Number of the current mapping pages. */ + xen_ulong_t nr_pages; +}; +typedef struct xen_mem_reserved_device_memory xen_mem_reserved_device_memory_t; +DEFINE_XEN_GUEST_HANDLE(xen_mem_reserved_device_memory_t); + +struct xen_mem_reserved_device_memory_map { + /* + * On call the number of entries which can be stored in buffer. On + * return the number of entries which have been stored in + * buffer. + */ + unsigned int nr_entries; + + /* + * xen_mem_reserved_device_memory entries in the buffer. + */ + XEN_GUEST_HANDLE(xen_mem_reserved_device_memory_t) buffer; +}; +typedef struct xen_mem_reserved_device_memory_map xen_mem_reserved_device_memory_map_t; +DEFINE_XEN_GUEST_HANDLE(xen_mem_reserved_device_memory_map_t); + +/* Next available subop number is 27 */ #endif /* __XEN_PUBLIC_MEMORY_H__ */ diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h index b183189..4fceafd 100644 --- a/xen/include/xen/mm.h +++ b/xen/include/xen/mm.h @@ -31,6 +31,7 @@ #include #include #include +#include struct domain; struct page_info; @@ -371,4 +372,6 @@ int guest_remove_page(struct domain *d, unsigned long gmfn); /* TRUE if the whole page at @mfn is of the requested RAM type(s) above. */ int page_is_ram_type(unsigned long mfn, unsigned long mem_type); +typedef int (*drmmap_callback_t)(struct xen_mem_reserved_device_memory_map *xmrdmmap); + #endif /* __XEN_MM_H__ */ -- 1.9.1 Thanks Tiejun