From mboxrd@z Thu Jan 1 00:00:00 1970 From: Olaf Hering Subject: Re: [PATCH] add HVMOP_get_mem_type hvmop Date: Tue, 3 May 2011 15:59:28 +0200 Message-ID: <20110503135928.GA7862@aepfle.de> References: <20110503133050.GE10252@whitby.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Return-path: Content-Disposition: inline In-Reply-To: <20110503133050.GE10252@whitby.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Tim Deegan Cc: "xen-devel@lists.xensource.com" List-Id: xen-devel@lists.xenproject.org On Tue, May 03, Tim Deegan wrote: > > + rc = rcu_lock_remote_target_domain_by_id(a.domid, &d); > > I thought this call was intended to be used from inside the guest in > question. rcu_lock_remote_target_domain_by_id() explicitly refuses to > let a domain operate on itself. Hmm, I have to test it with xen-unstable too. I did copy&paste from other parts of the file. 4.0 used rcu_lock_target_domain_by_id, so can I use that function? > > +typedef enum { > > + HVMMEM_ram_rw, /* Normal read/write guest RAM */ > > + HVMMEM_ram_ro, /* Read-only; writes are discarded */ > > + HVMMEM_mmio_dm, /* Reads and write go to the device model */ > > +} hvmmem_type_t; > > + > > This is now outside the #ifdef, when both of its users are inside it. > If that wasn't deliberate, please put it back. Should HVMOP_get_mem_type use hvmmem_type_t? Its outside the ifdef, otherwise unmodified_drivers/linux-2.6/platform-pci/platform-pci.c does not get the define. > > /* Following tools-only interfaces may change in future. */ > > #if defined(__XEN__) || defined(__XEN_TOOLS__) > > > > @@ -109,11 +115,6 @@ typedef struct xen_hvm_modified_memory x > > DEFINE_XEN_GUEST_HANDLE(xen_hvm_modified_memory_t); > > > > #define HVMOP_set_mem_type 8 > > -typedef enum { > > - HVMMEM_ram_rw, /* Normal read/write guest RAM */ > > - HVMMEM_ram_ro, /* Read-only; writes are discarded */ > > - HVMMEM_mmio_dm, /* Reads and write go to the device model */ > > -} hvmmem_type_t; > > /* Notify that a region of memory is to be treated in a specific way. */ > > struct xen_hvm_set_mem_type { > > /* Domain to be updated. */ > > @@ -223,6 +224,20 @@ struct xen_hvm_inject_trap { > > typedef struct xen_hvm_inject_trap xen_hvm_inject_trap_t; > > DEFINE_XEN_GUEST_HANDLE(xen_hvm_inject_trap_t); > > > > +#define HVMOP_get_mem_type 15 > > +/* Return hvmmem_type_t for the specified pfn. */ > > +struct xen_hvm_get_mem_type { > > + /* Domain to be queried. */ > > + domid_t domid; > > + /* OUT variable. */ > > + uint8_t mem_type; > > + /* IN variable. */ > > + uint64_t pfn; > > This structure will be laid out differently on 32-bit and 64-bit > builds. :( Also, since the _set operation uses a 16-bit variable for > the type, you might as well do the same here. Good point. Should I use the same padding as in xen_hvm_pagetable_dying? struct xen_hvm_get_mem_type { /* Domain to be queried. */ domid_t domid; /* OUT variable. */ uint16_t mem_type; uint16_t pad[2]; /* IN variable. */ uint64_t pfn; }; Olaf