* Re: [PATCH 01/18] xen: reinstate previously unused XENMEM_remove_from_physmap hypercall [not found] <mailman.745.1326901231.1471.xen-devel@lists.xensource.com> @ 2012-01-18 15:47 ` Andres Lagar-Cavilla 0 siblings, 0 replies; 11+ messages in thread From: Andres Lagar-Cavilla @ 2012-01-18 15:47 UTC (permalink / raw) To: xen-devel; +Cc: Daniel De Graaf, Alex Zeffertt, Ian Campbell > Send Xen-devel mailing list submissions to > xen-devel@lists.xensource.com > > To subscribe or unsubscribe via the World Wide Web, visit > http://lists.xensource.com/mailman/listinfo/xen-devel > or, via email, send a message with subject or body 'help' to > xen-devel-request@lists.xensource.com > > You can reach the person managing the list at > xen-devel-owner@lists.xensource.com > > When replying, please edit your Subject line so it is more specific > than "Re: Contents of Xen-devel digest..." > > Date: Wed, 18 Jan 2012 07:40:22 -0800 > From: "Andres Lagar-Cavilla" <andres@lagarcavilla.org> > To: xen-devel@lists.xensource.com > Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>, Alex Zeffertt > <alex.zeffertt@eu.citrix.com>, Ian Campbell <Ian.Campbell@citrix.com> > Subject: Re: [Xen-devel] [PATCH 01/18] xen: reinstate previously > unused XENMEM_remove_from_physmap hypercall > Message-ID: > <e1111841be4c16c1c4ebfbee4bc8ed03.squirrel@webmail.lagarcavilla.org> > Content-Type: text/plain;charset=iso-8859-1 > >> Date: Wed, 18 Jan 2012 10:36:07 +0000 >> From: Ian Campbell <Ian.Campbell@citrix.com> >> To: Daniel De Graaf <dgdegra@tycho.nsa.gov> >> Cc: Alex Zeffertt <alex.zeffertt@eu.citrix.com>, >> "xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com> >> Subject: Re: [Xen-devel] [PATCH 01/18] xen: reinstate previously >> unused XENMEM_remove_from_physmap hypercall >> Message-ID: <1326882968.14689.176.camel@zakaz.uk.xensource.com> >> Content-Type: text/plain; charset="UTF-8" >> >> On Thu, 2012-01-12 at 23:35 +0000, Daniel De Graaf wrote: >>> From: Alex Zeffertt <alex.zeffertt@eu.citrix.com> >>> >>> This patch reinstates the XENMEM_remove_from_physmap hypercall >>> which was removed in 19041:ee62aaafff46 because it was not used. >>> >>> However, is now needed in order to support xenstored stub domains. >>> The xenstored stub domain is not priviliged like dom0 and so cannot >>> unilaterally map the xenbus page of other guests into it's address >>> space. Therefore, before creating a domU the domain builder needs to >>> seed its grant table with a grant ref allowing the xenstored stub >>> domain to access the new domU's xenbus page. >>> >>> At present domU's do not start with their grant table mapped. >>> Instead it gets mapped when the guest requests a grant table from >>> the hypervisor. >>> >>> In order to seed the grant table, the domain builder first needs to >>> map it into dom0 address space. But the hypercall to do this >>> requires a gpfn (guest pfn), which is an mfn for PV guest, but a pfn >>> for HVM guests. Therfore, in order to seed the grant table of an >>> HVM guest, dom0 needs to *temporarily* map it into the guest's >>> "physical" address space. >>> >>> Hence the need to reinstate the XENMEM_remove_from_physmap hypercall. >>> >>> Signed-off-by: Alex Zeffertt <alex.zeffertt@eu.citrix.com> >>> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> >> >> Acked-by: Ian Campbell <ian.campbell@citrix.com> (modulo Jan's comment >> about ordering in xlat.lst) >> >> BTW, since Alex and Diego have subsequently left Citrix you could take >> my Acked-by's in this series as Signed-of-by on behalf of Citrix. I've >> no idea if that's necessary though, I expect not. >> >> Ian. > > Nacked-by-me for a couple of reasons, see inline below: Oh wow, spoke way too soon. It's very much correct. Ignore my spam please. Andres > >> >>> --- >>> xen/arch/ia64/xen/mm.c | 34 >>> ++++++++++++++++++++++++++++++++++ >>> xen/arch/x86/mm.c | 35 >>> +++++++++++++++++++++++++++++++++++ >>> xen/arch/x86/x86_64/compat/mm.c | 14 ++++++++++++++ >>> xen/include/public/memory.h | 16 ++++++++++++++++ >>> xen/include/xlat.lst | 1 + >>> xen/include/xsm/xsm.h | 6 ++++++ >>> xen/xsm/dummy.c | 6 ++++++ >>> xen/xsm/flask/hooks.c | 6 ++++++ >>> 8 files changed, 118 insertions(+), 0 deletions(-) >>> >>> diff --git a/xen/arch/ia64/xen/mm.c b/xen/arch/ia64/xen/mm.c >>> index 84f6a61..a40f96a 100644 >>> --- a/xen/arch/ia64/xen/mm.c >>> +++ b/xen/arch/ia64/xen/mm.c >>> @@ -3448,6 +3448,40 @@ arch_memory_op(int op, XEN_GUEST_HANDLE(void) >>> arg) >>> break; >>> } >>> >>> + case XENMEM_remove_from_physmap: >>> + { >>> + struct xen_remove_from_physmap xrfp; >>> + unsigned long mfn; >>> + struct domain *d; >>> + >>> + if ( copy_from_guest(&xrfp, arg, 1) ) >>> + return -EFAULT; >>> + >>> + rc = rcu_lock_target_domain_by_id(xrfp.domid, &d); >>> + if ( rc != 0 ) >>> + return rc; >>> + >>> + if ( xsm_remove_from_physmap(current->domain, d) ) >>> + { >>> + rcu_unlock_domain(d); >>> + return -EPERM; >>> + } >>> + >>> + domain_lock(d); >>> + >>> + mfn = gmfn_to_mfn(d, xrfp.gpfn); > > Compilation will fail. gmfn_to_mfn has been deprecated in x86. You need to > wrap with an ifdef (ia64 still uses gmfn_to_mfn), and in the x86 case use > get_gfn_untyped. > >>> + >>> + if ( mfn_valid(mfn) ) >>> + guest_physmap_remove_page(d, xrfp.gpfn, mfn, 0); >>> + > And, you need to invoke put_gfn on your way out (no ifdef, ia64 has the > stub) > > Thanks! > Andres >>> + domain_unlock(d); >>> + >>> + rcu_unlock_domain(d); >>> + >>> + break; >>> + } >>> + >>> + >>> case XENMEM_machine_memory_map: >>> { >>> struct xen_memory_map memmap; >>> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c >>> index 1f996e0..39cc3c0 100644 >>> --- a/xen/arch/x86/mm.c >>> +++ b/xen/arch/x86/mm.c >>> @@ -4871,6 +4871,41 @@ long arch_memory_op(int op, >>> XEN_GUEST_HANDLE(void) arg) >>> return rc; >>> } >>> >>> + case XENMEM_remove_from_physmap: >>> + { >>> + struct xen_remove_from_physmap xrfp; >>> + unsigned long mfn; >>> + struct domain *d; >>> + >>> + if ( copy_from_guest(&xrfp, arg, 1) ) >>> + return -EFAULT; >>> + >>> + rc = rcu_lock_target_domain_by_id(xrfp.domid, &d); >>> + if ( rc != 0 ) >>> + return rc; >>> + >>> + if ( xsm_remove_from_physmap(current->domain, d) ) >>> + { >>> + rcu_unlock_domain(d); >>> + return -EPERM; >>> + } >>> + >>> + domain_lock(d); >>> + >>> + mfn = get_gfn_untyped(d, xrfp.gpfn); >>> + >>> + if ( mfn_valid(mfn) ) >>> + guest_physmap_remove_page(d, xrfp.gpfn, mfn, >>> PAGE_ORDER_4K); >>> + >>> + put_gfn(d, xrfp.gpfn); >>> + >>> + domain_unlock(d); >>> + >>> + rcu_unlock_domain(d); >>> + >>> + break; >>> + } >>> + >>> case XENMEM_set_memory_map: >>> { >>> struct xen_foreign_memory_map fmap; >>> diff --git a/xen/arch/x86/x86_64/compat/mm.c >>> b/xen/arch/x86/x86_64/compat/mm.c >>> index bea94fe..dde5430 100644 >>> --- a/xen/arch/x86/x86_64/compat/mm.c >>> +++ b/xen/arch/x86/x86_64/compat/mm.c >>> @@ -82,6 +82,20 @@ int compat_arch_memory_op(int op, >>> XEN_GUEST_HANDLE(void) arg) >>> break; >>> } >>> >>> + case XENMEM_remove_from_physmap: >>> + { >>> + struct compat_remove_from_physmap cmp; >>> + struct xen_remove_from_physmap *nat = (void >>> *)COMPAT_ARG_XLAT_VIRT_BASE; >>> + >>> + if ( copy_from_guest(&cmp, arg, 1) ) >>> + return -EFAULT; >>> + >>> + XLAT_remove_from_physmap(nat, &cmp); >>> + rc = arch_memory_op(op, guest_handle_from_ptr(nat, void)); >>> + >>> + break; >>> + } >>> + >>> case XENMEM_set_memory_map: >>> { >>> struct compat_foreign_memory_map cmp; >>> diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h >>> index c5b78a8..308deff 100644 >>> --- a/xen/include/public/memory.h >>> +++ b/xen/include/public/memory.h >>> @@ -229,6 +229,22 @@ struct xen_add_to_physmap { >>> typedef struct xen_add_to_physmap xen_add_to_physmap_t; >>> DEFINE_XEN_GUEST_HANDLE(xen_add_to_physmap_t); >>> >>> +/* >>> + * Unmaps the page appearing at a particular GPFN from the specified >>> guest's >>> + * pseudophysical address space. >>> + * arg == addr of xen_remove_from_physmap_t. >>> + */ >>> +#define XENMEM_remove_from_physmap 15 >>> +struct xen_remove_from_physmap { >>> + /* Which domain to change the mapping for. */ >>> + domid_t domid; >>> + >>> + /* GPFN of the current mapping of the page. */ >>> + xen_pfn_t gpfn; >>> +}; >>> +typedef struct xen_remove_from_physmap xen_remove_from_physmap_t; >>> +DEFINE_XEN_GUEST_HANDLE(xen_remove_from_physmap_t); >>> + >>> /*** REMOVED ***/ >>> /*#define XENMEM_translate_gpfn_list 8*/ >>> >>> diff --git a/xen/include/xlat.lst b/xen/include/xlat.lst >>> index 3d92175..ee1772c 100644 >>> --- a/xen/include/xlat.lst >>> +++ b/xen/include/xlat.lst >>> @@ -81,6 +81,7 @@ >>> ! processor_power platform.h >>> ? processor_px platform.h >>> ! psd_package platform.h >>> +! remove_from_physmap memory.h >>> ? xenpf_pcpuinfo platform.h >>> ? xenpf_pcpu_version platform.h >>> ! sched_poll sched.h >>> diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h >>> index df6cec2..566c808 100644 >>> --- a/xen/include/xsm/xsm.h >>> +++ b/xen/include/xsm/xsm.h >>> @@ -169,6 +169,7 @@ struct xsm_operations { >>> int (*update_va_mapping) (struct domain *d, struct domain *f, >>> l1_pgentry_t >>> pte); >>> int (*add_to_physmap) (struct domain *d1, struct domain *d2); >>> + int (*remove_from_physmap) (struct domain *d1, struct domain *d2); >>> int (*sendtrigger) (struct domain *d); >>> int (*bind_pt_irq) (struct domain *d, struct >>> xen_domctl_bind_pt_irq >>> *bind); >>> int (*unbind_pt_irq) (struct domain *d); >>> @@ -738,6 +739,11 @@ static inline int xsm_add_to_physmap(struct domain >>> *d1, struct domain *d2) >>> return xsm_call(add_to_physmap(d1, d2)); >>> } >>> >>> +static inline int xsm_remove_from_physmap(struct domain *d1, struct >>> domain *d2) >>> +{ >>> + return xsm_call(remove_from_physmap(d1, d2)); >>> +} >>> + >>> static inline int xsm_sendtrigger(struct domain *d) >>> { >>> return xsm_call(sendtrigger(d)); >>> diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c >>> index 4bbfbff..65daa4e 100644 >>> --- a/xen/xsm/dummy.c >>> +++ b/xen/xsm/dummy.c >>> @@ -529,6 +529,11 @@ static int dummy_add_to_physmap (struct domain >>> *d1, >>> struct domain *d2) >>> return 0; >>> } >>> >>> +static int dummy_remove_from_physmap (struct domain *d1, struct domain >>> *d2) >>> +{ >>> + return 0; >>> +} >>> + >>> static int dummy_sendtrigger (struct domain *d) >>> { >>> return 0; >>> @@ -690,6 +695,7 @@ void xsm_fixup_ops (struct xsm_operations *ops) >>> set_to_dummy_if_null(ops, mmu_machphys_update); >>> set_to_dummy_if_null(ops, update_va_mapping); >>> set_to_dummy_if_null(ops, add_to_physmap); >>> + set_to_dummy_if_null(ops, remove_from_physmap); >>> set_to_dummy_if_null(ops, sendtrigger); >>> set_to_dummy_if_null(ops, bind_pt_irq); >>> set_to_dummy_if_null(ops, pin_mem_cacheattr); >>> diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c >>> index 0d35767..a2020a9 100644 >>> --- a/xen/xsm/flask/hooks.c >>> +++ b/xen/xsm/flask/hooks.c >>> @@ -1283,6 +1283,11 @@ static int flask_add_to_physmap(struct domain >>> *d1, struct domain *d2) >>> return domain_has_perm(d1, d2, SECCLASS_MMU, MMU__PHYSMAP); >>> } >>> >>> +static int flask_remove_from_physmap(struct domain *d1, struct domain >>> *d2) >>> +{ >>> + return domain_has_perm(d1, d2, SECCLASS_MMU, MMU__PHYSMAP); >>> +} >>> + >>> static int flask_sendtrigger(struct domain *d) >>> { >>> return domain_has_perm(current->domain, d, SECCLASS_DOMAIN, >>> DOMAIN__TRIGGER); >>> @@ -1550,6 +1555,7 @@ static struct xsm_operations flask_ops = { >>> .mmu_machphys_update = flask_mmu_machphys_update, >>> .update_va_mapping = flask_update_va_mapping, >>> .add_to_physmap = flask_add_to_physmap, >>> + .remove_from_physmap = flask_remove_from_physmap, >>> .sendtrigger = flask_sendtrigger, >>> .get_device_group = flask_get_device_group, >>> .test_assign_device = flask_test_assign_device, >> >> >> >> >> >> ------------------------------ >> >> Message: 3 >> Date: Wed, 18 Jan 2012 11:40:06 +0100 >> From: Wei Wang <wei.wang2@amd.com> >> To: Dario Faggioli <raistlin@linux.it> >> Cc: Tim Deegan <tim@xen.org>, "allen.m.kay@intel.com" >> <allen.m.kay@intel.com>, xen-devel@lists.xensource.com, Keir Fraser >> <keir@xen.org>, Jan Beulich <JBeulich@suse.com> >> Subject: Re: [Xen-devel] [PATCHv2 2 of 2] Move IOMMU faults handling >> into softirq for AMD-Vi. >> Message-ID: <4F16A186.4080303@amd.com> >> Content-Type: text/plain; charset="UTF-8"; format=flowed >> >> On 01/18/2012 09:53 AM, Dario Faggioli wrote: >>> On Tue, 2012-01-17 at 11:17 +0000, Keir Fraser wrote: >>>>> Dealing with interrupts from AMD-Vi IOMMU(s) is deferred to a >>>>> softirq-tasklet, >>>>> raised by the actual IRQ handler. To avoid more interrupts being >>>>> generated >>>>> (because of further faults), they must be masked in the IOMMU within >>>>> the low >>>>> level IRQ handler and enabled back in the tasklet body. Notice that >>>>> this may >>>>> cause the log to overflow, but none of the existing entry will be >>>>> overwritten. >>>>> >>>>> Signed-off-by: Dario Faggioli<dario.faggioli@citrix.com> >>>> >>>> This patch needs fixing to apply to xen-unstable tip. Please do that >>>> and >>>> resubmit. >>>> >>> I see. I can easily rebase the patch but there are functional changes >>> involved, so I'd like to know what you think it's best to do first. >>> >>> In particular, the clash is against Wei's patches introducing PPR. So >>> now the IOMMU interrupt handler checks both event log and ppr log. >>> >>> Question is, should I move _BOTH_ these checks into softirq or just >>> defer event log processing, and leave ppr log handling in hard-irq >>> context? Quickly looking at the new specs, it seems to me that >>> deferring >>> both should be fine, but I'd really appreciate your thoughts... >> >> I think put both event log and ppr log into softirq is fine. If you >> could have a patch like this, I can do a quick test on my machine. >> Thanks, >> Wei >> >>> Wei, Jan, Tim? >>> >>> Thanks and regards, >>> Dario >>> >> >> >> >> >> >> ------------------------------ >> >> Message: 4 >> Date: Wed, 18 Jan 2012 10:39:01 +0000 >> From: Ian Campbell <Ian.Campbell@citrix.com> >> To: Daniel De Graaf <dgdegra@tycho.nsa.gov> >> Cc: Alex Zeffertt <alex.zeffertt@eu.citrix.com>, >> "xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>, Diego >> Ongaro <diego.ongaro@citrix.com> >> Subject: Re: [Xen-devel] [PATCH 02/18] xen: allow global VIRQ handlers >> to be delegated to other domains >> Message-ID: <1326883141.14689.177.camel@zakaz.uk.xensource.com> >> Content-Type: text/plain; charset="UTF-8" >> >> On Thu, 2012-01-12 at 23:35 +0000, Daniel De Graaf wrote: >>> >>> +static void clear_global_virq_handlers(struct domain *d) >>> +{ >>> + uint32_t virq; >>> + int put_count = 0; >>> + >>> + spin_lock(&global_virq_handlers_lock); >>> + >>> + for (virq = 0; virq < NR_VIRQS; virq++) { >>> + if (global_virq_handlers[virq] == d) { >>> + global_virq_handlers[virq] = NULL; >> >> I don't suppose we should rebind to dom0, should we? >> >> Seems like we are pretty hosed if this ever happens in a non-controlled >> manner anyway... >> >>> + put_count++; >>> + } >>> + } >>> + >>> + spin_unlock(&global_virq_handlers_lock); >>> + >>> + while (put_count) { >>> + put_domain(d); >>> + put_count--; >>> + } >>> +} >> >> >> >> >> ------------------------------ >> >> Message: 5 >> Date: Wed, 18 Jan 2012 11:39:22 +0100 >> From: Juergen Gross <juergen.gross@ts.fujitsu.com> >> To: Keir Fraser <keir.xen@gmail.com> >> Cc: xen-devel@lists.xensource.com >> Subject: Re: [Xen-devel] [PATCH] Allow wake up of offline vcpu via >> nmi-ipi >> Message-ID: <4F16A15A.3040405@ts.fujitsu.com> >> Content-Type: text/plain; charset=ISO-8859-1; format=flowed >> >> On 01/18/2012 10:36 AM, Keir Fraser wrote: >>> On 18/01/2012 09:31, "Keir Fraser"<keir.xen@gmail.com> wrote: >>> >>>> On 18/01/2012 09:07, "Juergen Gross"<juergen.gross@ts.fujitsu.com> >>>> wrote: >>>> >>>>> On 01/18/2012 09:48 AM, Juergen Gross wrote: >>>>>> On a real machine a cpu disabled via hlt with interrupts disabled >>>>>> can >>>>>> be >>>>>> reactivated via a nmi ipi. Enable the hypervisor to do this for hvm, >>>>>> too. >>>>>> >>>>>> Signed-off-by: juergen.gross@ts.fujitsu.com >>>>>> >>>>>> >>>>>> 1 file changed, 4 insertions(+), 1 deletion(-) >>>>>> xen/arch/x86/hvm/vlapic.c | 5 ++++- >>>>> BTW: I was not able to reactivate a vcpu via INIT/SIPI/SIPI sequence. >>>>> It >>>>> works >>>>> on initial system boot when the target vcpu is activated the first >>>>> time. If I >>>>> deactivate a vcpu and try to activate it again it will start to run, >>>>> but it >>>>> is >>>>> not starting at the specified entry point (at least it isn't >>>>> performing the >>>>> first instruction there). >>>>> Is there some special initialization needed to make this work? Do I >>>>> have to >>>>> reset >>>>> something on the vcpu before deactivating it? >>>> No it should just work. Hvmloader wakes and then sleeps every AP, in >>>> hvmloader/smp.c. So even the first INIT-SIPI wakeup of an AP in the >>>> guest OS >>>> is not the first, as hvmloader already did it once! So this path >>>> should >>>> be >>>> working and indeed tested on every HVM guest boot. >>> Bit more info: INIT-SIPI logic is complicated by needing to avoid >>> deadlocks >>> between two VCPUs attempting to pause and reset each other. But the >>> core >>> dispatch logic is in vlapic_init_sipi_action(). You will see that on >>> INIT, >>> we should call vcpu_reset() which will de-initialise and VCPU_down the >>> vcpu. >>> And then on SIPI we call hvm_vcpu_reset_state(), which should >>> reinitialise >>> and wake the vcpu to start running at the specified CS:IP. >>> >>> So the above will be good places for you to add tracing and work out >>> what's >>> going on. :-) >> >> Yeah, thanks for the confirmation this should work. >> Printing some diagnostics helped me to spot the error in my code. >> >> >> Juergen >> >> -- >> Juergen Gross Principal Developer Operating Systems >> PDG ES&S SWE OS6 Telephone: +49 (0) 89 3222 2967 >> Fujitsu Technology Solutions e-mail: >> juergen.gross@ts.fujitsu.com >> Domagkstr. 28 Internet: ts.fujitsu.com >> D-80807 Muenchen Company details: >> ts.fujitsu.com/imprint.html >> >> >> >> >> ------------------------------ >> >> _______________________________________________ >> Xen-devel mailing list >> Xen-devel@lists.xensource.com >> http://lists.xensource.com/xen-devel >> >> >> End of Xen-devel Digest, Vol 83, Issue 277 >> ****************************************** >> > > > > > > ------------------------------ > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel > > > End of Xen-devel Digest, Vol 83, Issue 284 > ****************************************** > ^ permalink raw reply [flat|nested] 11+ messages in thread
[parent not found: <mailman.700.1326883173.1471.xen-devel@lists.xensource.com>]
* Re: [PATCH 01/18] xen: reinstate previously unused XENMEM_remove_from_physmap hypercall [not found] <mailman.700.1326883173.1471.xen-devel@lists.xensource.com> @ 2012-01-18 15:40 ` Andres Lagar-Cavilla 0 siblings, 0 replies; 11+ messages in thread From: Andres Lagar-Cavilla @ 2012-01-18 15:40 UTC (permalink / raw) To: xen-devel; +Cc: Daniel De Graaf, Alex Zeffertt, Ian Campbell > Date: Wed, 18 Jan 2012 10:36:07 +0000 > From: Ian Campbell <Ian.Campbell@citrix.com> > To: Daniel De Graaf <dgdegra@tycho.nsa.gov> > Cc: Alex Zeffertt <alex.zeffertt@eu.citrix.com>, > "xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com> > Subject: Re: [Xen-devel] [PATCH 01/18] xen: reinstate previously > unused XENMEM_remove_from_physmap hypercall > Message-ID: <1326882968.14689.176.camel@zakaz.uk.xensource.com> > Content-Type: text/plain; charset="UTF-8" > > On Thu, 2012-01-12 at 23:35 +0000, Daniel De Graaf wrote: >> From: Alex Zeffertt <alex.zeffertt@eu.citrix.com> >> >> This patch reinstates the XENMEM_remove_from_physmap hypercall >> which was removed in 19041:ee62aaafff46 because it was not used. >> >> However, is now needed in order to support xenstored stub domains. >> The xenstored stub domain is not priviliged like dom0 and so cannot >> unilaterally map the xenbus page of other guests into it's address >> space. Therefore, before creating a domU the domain builder needs to >> seed its grant table with a grant ref allowing the xenstored stub >> domain to access the new domU's xenbus page. >> >> At present domU's do not start with their grant table mapped. >> Instead it gets mapped when the guest requests a grant table from >> the hypervisor. >> >> In order to seed the grant table, the domain builder first needs to >> map it into dom0 address space. But the hypercall to do this >> requires a gpfn (guest pfn), which is an mfn for PV guest, but a pfn >> for HVM guests. Therfore, in order to seed the grant table of an >> HVM guest, dom0 needs to *temporarily* map it into the guest's >> "physical" address space. >> >> Hence the need to reinstate the XENMEM_remove_from_physmap hypercall. >> >> Signed-off-by: Alex Zeffertt <alex.zeffertt@eu.citrix.com> >> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> > > Acked-by: Ian Campbell <ian.campbell@citrix.com> (modulo Jan's comment > about ordering in xlat.lst) > > BTW, since Alex and Diego have subsequently left Citrix you could take > my Acked-by's in this series as Signed-of-by on behalf of Citrix. I've > no idea if that's necessary though, I expect not. > > Ian. Nacked-by-me for a couple of reasons, see inline below: > >> --- >> xen/arch/ia64/xen/mm.c | 34 >> ++++++++++++++++++++++++++++++++++ >> xen/arch/x86/mm.c | 35 >> +++++++++++++++++++++++++++++++++++ >> xen/arch/x86/x86_64/compat/mm.c | 14 ++++++++++++++ >> xen/include/public/memory.h | 16 ++++++++++++++++ >> xen/include/xlat.lst | 1 + >> xen/include/xsm/xsm.h | 6 ++++++ >> xen/xsm/dummy.c | 6 ++++++ >> xen/xsm/flask/hooks.c | 6 ++++++ >> 8 files changed, 118 insertions(+), 0 deletions(-) >> >> diff --git a/xen/arch/ia64/xen/mm.c b/xen/arch/ia64/xen/mm.c >> index 84f6a61..a40f96a 100644 >> --- a/xen/arch/ia64/xen/mm.c >> +++ b/xen/arch/ia64/xen/mm.c >> @@ -3448,6 +3448,40 @@ arch_memory_op(int op, XEN_GUEST_HANDLE(void) >> arg) >> break; >> } >> >> + case XENMEM_remove_from_physmap: >> + { >> + struct xen_remove_from_physmap xrfp; >> + unsigned long mfn; >> + struct domain *d; >> + >> + if ( copy_from_guest(&xrfp, arg, 1) ) >> + return -EFAULT; >> + >> + rc = rcu_lock_target_domain_by_id(xrfp.domid, &d); >> + if ( rc != 0 ) >> + return rc; >> + >> + if ( xsm_remove_from_physmap(current->domain, d) ) >> + { >> + rcu_unlock_domain(d); >> + return -EPERM; >> + } >> + >> + domain_lock(d); >> + >> + mfn = gmfn_to_mfn(d, xrfp.gpfn); Compilation will fail. gmfn_to_mfn has been deprecated in x86. You need to wrap with an ifdef (ia64 still uses gmfn_to_mfn), and in the x86 case use get_gfn_untyped. >> + >> + if ( mfn_valid(mfn) ) >> + guest_physmap_remove_page(d, xrfp.gpfn, mfn, 0); >> + And, you need to invoke put_gfn on your way out (no ifdef, ia64 has the stub) Thanks! Andres >> + domain_unlock(d); >> + >> + rcu_unlock_domain(d); >> + >> + break; >> + } >> + >> + >> case XENMEM_machine_memory_map: >> { >> struct xen_memory_map memmap; >> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c >> index 1f996e0..39cc3c0 100644 >> --- a/xen/arch/x86/mm.c >> +++ b/xen/arch/x86/mm.c >> @@ -4871,6 +4871,41 @@ long arch_memory_op(int op, >> XEN_GUEST_HANDLE(void) arg) >> return rc; >> } >> >> + case XENMEM_remove_from_physmap: >> + { >> + struct xen_remove_from_physmap xrfp; >> + unsigned long mfn; >> + struct domain *d; >> + >> + if ( copy_from_guest(&xrfp, arg, 1) ) >> + return -EFAULT; >> + >> + rc = rcu_lock_target_domain_by_id(xrfp.domid, &d); >> + if ( rc != 0 ) >> + return rc; >> + >> + if ( xsm_remove_from_physmap(current->domain, d) ) >> + { >> + rcu_unlock_domain(d); >> + return -EPERM; >> + } >> + >> + domain_lock(d); >> + >> + mfn = get_gfn_untyped(d, xrfp.gpfn); >> + >> + if ( mfn_valid(mfn) ) >> + guest_physmap_remove_page(d, xrfp.gpfn, mfn, >> PAGE_ORDER_4K); >> + >> + put_gfn(d, xrfp.gpfn); >> + >> + domain_unlock(d); >> + >> + rcu_unlock_domain(d); >> + >> + break; >> + } >> + >> case XENMEM_set_memory_map: >> { >> struct xen_foreign_memory_map fmap; >> diff --git a/xen/arch/x86/x86_64/compat/mm.c >> b/xen/arch/x86/x86_64/compat/mm.c >> index bea94fe..dde5430 100644 >> --- a/xen/arch/x86/x86_64/compat/mm.c >> +++ b/xen/arch/x86/x86_64/compat/mm.c >> @@ -82,6 +82,20 @@ int compat_arch_memory_op(int op, >> XEN_GUEST_HANDLE(void) arg) >> break; >> } >> >> + case XENMEM_remove_from_physmap: >> + { >> + struct compat_remove_from_physmap cmp; >> + struct xen_remove_from_physmap *nat = (void >> *)COMPAT_ARG_XLAT_VIRT_BASE; >> + >> + if ( copy_from_guest(&cmp, arg, 1) ) >> + return -EFAULT; >> + >> + XLAT_remove_from_physmap(nat, &cmp); >> + rc = arch_memory_op(op, guest_handle_from_ptr(nat, void)); >> + >> + break; >> + } >> + >> case XENMEM_set_memory_map: >> { >> struct compat_foreign_memory_map cmp; >> diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h >> index c5b78a8..308deff 100644 >> --- a/xen/include/public/memory.h >> +++ b/xen/include/public/memory.h >> @@ -229,6 +229,22 @@ struct xen_add_to_physmap { >> typedef struct xen_add_to_physmap xen_add_to_physmap_t; >> DEFINE_XEN_GUEST_HANDLE(xen_add_to_physmap_t); >> >> +/* >> + * Unmaps the page appearing at a particular GPFN from the specified >> guest's >> + * pseudophysical address space. >> + * arg == addr of xen_remove_from_physmap_t. >> + */ >> +#define XENMEM_remove_from_physmap 15 >> +struct xen_remove_from_physmap { >> + /* Which domain to change the mapping for. */ >> + domid_t domid; >> + >> + /* GPFN of the current mapping of the page. */ >> + xen_pfn_t gpfn; >> +}; >> +typedef struct xen_remove_from_physmap xen_remove_from_physmap_t; >> +DEFINE_XEN_GUEST_HANDLE(xen_remove_from_physmap_t); >> + >> /*** REMOVED ***/ >> /*#define XENMEM_translate_gpfn_list 8*/ >> >> diff --git a/xen/include/xlat.lst b/xen/include/xlat.lst >> index 3d92175..ee1772c 100644 >> --- a/xen/include/xlat.lst >> +++ b/xen/include/xlat.lst >> @@ -81,6 +81,7 @@ >> ! processor_power platform.h >> ? processor_px platform.h >> ! psd_package platform.h >> +! remove_from_physmap memory.h >> ? xenpf_pcpuinfo platform.h >> ? xenpf_pcpu_version platform.h >> ! sched_poll sched.h >> diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h >> index df6cec2..566c808 100644 >> --- a/xen/include/xsm/xsm.h >> +++ b/xen/include/xsm/xsm.h >> @@ -169,6 +169,7 @@ struct xsm_operations { >> int (*update_va_mapping) (struct domain *d, struct domain *f, >> l1_pgentry_t >> pte); >> int (*add_to_physmap) (struct domain *d1, struct domain *d2); >> + int (*remove_from_physmap) (struct domain *d1, struct domain *d2); >> int (*sendtrigger) (struct domain *d); >> int (*bind_pt_irq) (struct domain *d, struct xen_domctl_bind_pt_irq >> *bind); >> int (*unbind_pt_irq) (struct domain *d); >> @@ -738,6 +739,11 @@ static inline int xsm_add_to_physmap(struct domain >> *d1, struct domain *d2) >> return xsm_call(add_to_physmap(d1, d2)); >> } >> >> +static inline int xsm_remove_from_physmap(struct domain *d1, struct >> domain *d2) >> +{ >> + return xsm_call(remove_from_physmap(d1, d2)); >> +} >> + >> static inline int xsm_sendtrigger(struct domain *d) >> { >> return xsm_call(sendtrigger(d)); >> diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c >> index 4bbfbff..65daa4e 100644 >> --- a/xen/xsm/dummy.c >> +++ b/xen/xsm/dummy.c >> @@ -529,6 +529,11 @@ static int dummy_add_to_physmap (struct domain *d1, >> struct domain *d2) >> return 0; >> } >> >> +static int dummy_remove_from_physmap (struct domain *d1, struct domain >> *d2) >> +{ >> + return 0; >> +} >> + >> static int dummy_sendtrigger (struct domain *d) >> { >> return 0; >> @@ -690,6 +695,7 @@ void xsm_fixup_ops (struct xsm_operations *ops) >> set_to_dummy_if_null(ops, mmu_machphys_update); >> set_to_dummy_if_null(ops, update_va_mapping); >> set_to_dummy_if_null(ops, add_to_physmap); >> + set_to_dummy_if_null(ops, remove_from_physmap); >> set_to_dummy_if_null(ops, sendtrigger); >> set_to_dummy_if_null(ops, bind_pt_irq); >> set_to_dummy_if_null(ops, pin_mem_cacheattr); >> diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c >> index 0d35767..a2020a9 100644 >> --- a/xen/xsm/flask/hooks.c >> +++ b/xen/xsm/flask/hooks.c >> @@ -1283,6 +1283,11 @@ static int flask_add_to_physmap(struct domain >> *d1, struct domain *d2) >> return domain_has_perm(d1, d2, SECCLASS_MMU, MMU__PHYSMAP); >> } >> >> +static int flask_remove_from_physmap(struct domain *d1, struct domain >> *d2) >> +{ >> + return domain_has_perm(d1, d2, SECCLASS_MMU, MMU__PHYSMAP); >> +} >> + >> static int flask_sendtrigger(struct domain *d) >> { >> return domain_has_perm(current->domain, d, SECCLASS_DOMAIN, >> DOMAIN__TRIGGER); >> @@ -1550,6 +1555,7 @@ static struct xsm_operations flask_ops = { >> .mmu_machphys_update = flask_mmu_machphys_update, >> .update_va_mapping = flask_update_va_mapping, >> .add_to_physmap = flask_add_to_physmap, >> + .remove_from_physmap = flask_remove_from_physmap, >> .sendtrigger = flask_sendtrigger, >> .get_device_group = flask_get_device_group, >> .test_assign_device = flask_test_assign_device, > > > > > > ------------------------------ > > Message: 3 > Date: Wed, 18 Jan 2012 11:40:06 +0100 > From: Wei Wang <wei.wang2@amd.com> > To: Dario Faggioli <raistlin@linux.it> > Cc: Tim Deegan <tim@xen.org>, "allen.m.kay@intel.com" > <allen.m.kay@intel.com>, xen-devel@lists.xensource.com, Keir Fraser > <keir@xen.org>, Jan Beulich <JBeulich@suse.com> > Subject: Re: [Xen-devel] [PATCHv2 2 of 2] Move IOMMU faults handling > into softirq for AMD-Vi. > Message-ID: <4F16A186.4080303@amd.com> > Content-Type: text/plain; charset="UTF-8"; format=flowed > > On 01/18/2012 09:53 AM, Dario Faggioli wrote: >> On Tue, 2012-01-17 at 11:17 +0000, Keir Fraser wrote: >>>> Dealing with interrupts from AMD-Vi IOMMU(s) is deferred to a >>>> softirq-tasklet, >>>> raised by the actual IRQ handler. To avoid more interrupts being >>>> generated >>>> (because of further faults), they must be masked in the IOMMU within >>>> the low >>>> level IRQ handler and enabled back in the tasklet body. Notice that >>>> this may >>>> cause the log to overflow, but none of the existing entry will be >>>> overwritten. >>>> >>>> Signed-off-by: Dario Faggioli<dario.faggioli@citrix.com> >>> >>> This patch needs fixing to apply to xen-unstable tip. Please do that >>> and >>> resubmit. >>> >> I see. I can easily rebase the patch but there are functional changes >> involved, so I'd like to know what you think it's best to do first. >> >> In particular, the clash is against Wei's patches introducing PPR. So >> now the IOMMU interrupt handler checks both event log and ppr log. >> >> Question is, should I move _BOTH_ these checks into softirq or just >> defer event log processing, and leave ppr log handling in hard-irq >> context? Quickly looking at the new specs, it seems to me that deferring >> both should be fine, but I'd really appreciate your thoughts... > > I think put both event log and ppr log into softirq is fine. If you > could have a patch like this, I can do a quick test on my machine. > Thanks, > Wei > >> Wei, Jan, Tim? >> >> Thanks and regards, >> Dario >> > > > > > > ------------------------------ > > Message: 4 > Date: Wed, 18 Jan 2012 10:39:01 +0000 > From: Ian Campbell <Ian.Campbell@citrix.com> > To: Daniel De Graaf <dgdegra@tycho.nsa.gov> > Cc: Alex Zeffertt <alex.zeffertt@eu.citrix.com>, > "xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>, Diego > Ongaro <diego.ongaro@citrix.com> > Subject: Re: [Xen-devel] [PATCH 02/18] xen: allow global VIRQ handlers > to be delegated to other domains > Message-ID: <1326883141.14689.177.camel@zakaz.uk.xensource.com> > Content-Type: text/plain; charset="UTF-8" > > On Thu, 2012-01-12 at 23:35 +0000, Daniel De Graaf wrote: >> >> +static void clear_global_virq_handlers(struct domain *d) >> +{ >> + uint32_t virq; >> + int put_count = 0; >> + >> + spin_lock(&global_virq_handlers_lock); >> + >> + for (virq = 0; virq < NR_VIRQS; virq++) { >> + if (global_virq_handlers[virq] == d) { >> + global_virq_handlers[virq] = NULL; > > I don't suppose we should rebind to dom0, should we? > > Seems like we are pretty hosed if this ever happens in a non-controlled > manner anyway... > >> + put_count++; >> + } >> + } >> + >> + spin_unlock(&global_virq_handlers_lock); >> + >> + while (put_count) { >> + put_domain(d); >> + put_count--; >> + } >> +} > > > > > ------------------------------ > > Message: 5 > Date: Wed, 18 Jan 2012 11:39:22 +0100 > From: Juergen Gross <juergen.gross@ts.fujitsu.com> > To: Keir Fraser <keir.xen@gmail.com> > Cc: xen-devel@lists.xensource.com > Subject: Re: [Xen-devel] [PATCH] Allow wake up of offline vcpu via > nmi-ipi > Message-ID: <4F16A15A.3040405@ts.fujitsu.com> > Content-Type: text/plain; charset=ISO-8859-1; format=flowed > > On 01/18/2012 10:36 AM, Keir Fraser wrote: >> On 18/01/2012 09:31, "Keir Fraser"<keir.xen@gmail.com> wrote: >> >>> On 18/01/2012 09:07, "Juergen Gross"<juergen.gross@ts.fujitsu.com> >>> wrote: >>> >>>> On 01/18/2012 09:48 AM, Juergen Gross wrote: >>>>> On a real machine a cpu disabled via hlt with interrupts disabled can >>>>> be >>>>> reactivated via a nmi ipi. Enable the hypervisor to do this for hvm, >>>>> too. >>>>> >>>>> Signed-off-by: juergen.gross@ts.fujitsu.com >>>>> >>>>> >>>>> 1 file changed, 4 insertions(+), 1 deletion(-) >>>>> xen/arch/x86/hvm/vlapic.c | 5 ++++- >>>> BTW: I was not able to reactivate a vcpu via INIT/SIPI/SIPI sequence. >>>> It >>>> works >>>> on initial system boot when the target vcpu is activated the first >>>> time. If I >>>> deactivate a vcpu and try to activate it again it will start to run, >>>> but it >>>> is >>>> not starting at the specified entry point (at least it isn't >>>> performing the >>>> first instruction there). >>>> Is there some special initialization needed to make this work? Do I >>>> have to >>>> reset >>>> something on the vcpu before deactivating it? >>> No it should just work. Hvmloader wakes and then sleeps every AP, in >>> hvmloader/smp.c. So even the first INIT-SIPI wakeup of an AP in the >>> guest OS >>> is not the first, as hvmloader already did it once! So this path should >>> be >>> working and indeed tested on every HVM guest boot. >> Bit more info: INIT-SIPI logic is complicated by needing to avoid >> deadlocks >> between two VCPUs attempting to pause and reset each other. But the core >> dispatch logic is in vlapic_init_sipi_action(). You will see that on >> INIT, >> we should call vcpu_reset() which will de-initialise and VCPU_down the >> vcpu. >> And then on SIPI we call hvm_vcpu_reset_state(), which should >> reinitialise >> and wake the vcpu to start running at the specified CS:IP. >> >> So the above will be good places for you to add tracing and work out >> what's >> going on. :-) > > Yeah, thanks for the confirmation this should work. > Printing some diagnostics helped me to spot the error in my code. > > > Juergen > > -- > Juergen Gross Principal Developer Operating Systems > PDG ES&S SWE OS6 Telephone: +49 (0) 89 3222 2967 > Fujitsu Technology Solutions e-mail: > juergen.gross@ts.fujitsu.com > Domagkstr. 28 Internet: ts.fujitsu.com > D-80807 Muenchen Company details: > ts.fujitsu.com/imprint.html > > > > > ------------------------------ > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel > > > End of Xen-devel Digest, Vol 83, Issue 277 > ****************************************** > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [RFC PATCH 0/18] Xenstore stub domain @ 2012-01-11 17:21 Daniel De Graaf 2012-01-11 17:21 ` [PATCH 01/18] xen: reinstate previously unused XENMEM_remove_from_physmap hypercall Daniel De Graaf 2012-01-12 23:35 ` [PATCH v2 00/18] Xenstore stub domain Daniel De Graaf 0 siblings, 2 replies; 11+ messages in thread From: Daniel De Graaf @ 2012-01-11 17:21 UTC (permalink / raw) To: xen-devel This patch series allows xenstored to run in a stub domian started by dom0. It is based on a patch series posted by Alex Zeffertt in 2009 - http://old-list-archives.xen.org/archives/html/xen-devel/2009-03/msg01488.html A domain configuration for starting xenstored looks like: kernel='/home/daniel/xen/stubdom/mini-os-x86_64-xenstore/mini-os' extra='' memory=50 name='xenstore' Once xenstore is started, "xenstore_dom=1" needs to be added to other domain's configurations in order to set up the xenstore connection to domain 1. The following program handles post-creation parts of xenstored. To use it, run "xl create -p xenstore" and then "init-xenstore $domid". The running xenstored must be stopped to prevent xl using the UNIX sockets, and xenconsoled needs to be restarted after switching xenstores. /* init-xenstore.c: link with -lxenctrl */ #include <fcntl.h> #include <stdio.h> #include <string.h> #include <stdint.h> #include <stdlib.h> #include <sys/ioctl.h> #include <sys/mman.h> #define __XEN_TOOLS__ #include <xen/domctl.h> #include "xenctrl.h" #define IOCTL_XENBUS_BACKEND_SETUP _IOC(_IOC_NONE, 'B', 1, 0) #define IOCTL_XENBUS_BACKEND_COMMIT _IOC(_IOC_NONE, 'B', 2, 0) static void set_virq(int domid, int virq) { struct xen_domctl command; xc_interface *xch; xch = xc_interface_open(NULL, NULL, 0); memset(&command, 0, sizeof(command)); command.cmd = XEN_DOMCTL_set_virq_handler; command.interface_version = XEN_DOMCTL_INTERFACE_VERSION; command.domain = domid; command.u.set_virq_handler.virq = virq; xc_domctl(xch, &command); xc_interface_close(xch); } int main(int argc, char** argv) { char buf[512]; int domid = atoi(argv[1]); set_virq(domid, VIRQ_DOM_EXC); int fd = open("/dev/xen/xenbus_backend", O_RDWR); void *map = mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0); int rv = ioctl(fd, IOCTL_XENBUS_BACKEND_SETUP, domid); *(uint16_t*)(map + 0x810) = rv; snprintf(buf, 512, "xl unpause %d", domid); system(buf); ioctl(fd, IOCTL_XENBUS_BACKEND_COMMIT, 0); return 0; } ------------------------------------------------- Dom0 kernel changes: [PATCH] xenbus: Add support for xenbus backend in stub domain This is based on the new /dev/xen devices introduced in Linux 3.3. Hypervisor changes: [PATCH 01/18] xen: reinstate previously unused [PATCH 02/18] xen: allow global VIRQ handlers to be delegated to [PATCH 03/18] xsm: allow use of XEN_DOMCTL_getdomaininfo by [PATCH 04/18] xen: Preserve reserved grant entries when switching Patch 1 & 4 are required for setting up grant entries in new domains. Patch 2 & 3 allow xenstored to run in an unprivileged domain. This currently requires XSM to be enabled to avoid allowing all domUs access to XEN_DOMCTL_getdomaininfo, so the patch only allows this hypercall if XSM is being compiled in. Toolstack changes: [PATCH 05/18] tools/libxl: Add xenstore and console backend domain [PATCH 06/18] lib{xc,xl}: Seed grant tables with xenstore and These patches populate two of the eight reserved grant entries in new domains with the xenstore and console shared pages, which is required if xenstored is not run in a privileged domain. Minios and xenstored: [PATCH 07/18] mini-os: avoid crash if no console is provided [PATCH 08/18] mini-os: avoid crash if no xenstore is provided [PATCH 09/18] mini-os: remove per-fd evtchn limit [PATCH 10/18] xenstored: use grant references instead of [PATCH 11/18] xenstored: add NO_SOCKETS compilation option [PATCH 12/18] xenstored support for in-memory rather than FS based [PATCH 13/18] xenstored: support running in minios stubdom [PATCH 14/18] xenstored: always use xc_gnttab_munmap in stubdom [PATCH 15/18] xenstored: add --event parameter for bootstrapping [PATCH 16/18] xenstored: pull dom0 event port from shared page [PATCH 17/18] xenstored: use domain_is_unprivileged instead of [PATCH 18/18] xenstored: add --priv-domid parameter Support for running in a stub domain ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 01/18] xen: reinstate previously unused XENMEM_remove_from_physmap hypercall 2012-01-11 17:21 [RFC PATCH 0/18] Xenstore stub domain Daniel De Graaf @ 2012-01-11 17:21 ` Daniel De Graaf 2012-01-12 8:22 ` Jan Beulich 2012-01-12 23:35 ` [PATCH v2 00/18] Xenstore stub domain Daniel De Graaf 1 sibling, 1 reply; 11+ messages in thread From: Daniel De Graaf @ 2012-01-11 17:21 UTC (permalink / raw) To: xen-devel; +Cc: Daniel De Graaf, Alex Zeffertt From: Alex Zeffertt <alex.zeffertt@eu.citrix.com> This patch reinstates the XENMEM_remove_from_physmap hypercall which was removed in 19041:ee62aaafff46 because it was not used. However, is now needed in order to support xenstored stub domains. The xenstored stub domain is not priviliged like dom0 and so cannot unilaterally map the xenbus page of other guests into it's address space. Therefore, before creating a domU the domain builder needs to seed its grant table with a grant ref allowing the xenstored stub domain to access the new domU's xenbus page. At present domU's do not start with their grant table mapped. Instead it gets mapped when the guest requests a grant table from the hypervisor. In order to seed the grant table, the domain builder first needs to map it into dom0 address space. But the hypercall to do this requires a gpfn (guest pfn), which is an mfn for PV guest, but a pfn for HVM guests. Therfore, in order to seed the grant table of an HVM guest, dom0 needs to *temporarily* map it into the guest's "physical" address space. Hence the need to reinstate the XENMEM_remove_from_physmap hypercall. Signed-off-by: Alex Zeffertt <alex.zeffertt@eu.citrix.com> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> --- xen/arch/ia64/xen/mm.c | 34 ++++++++++++++++++++++++++++++++++ xen/arch/x86/mm.c | 35 +++++++++++++++++++++++++++++++++++ xen/arch/x86/x86_64/compat/mm.c | 14 ++++++++++++++ xen/include/public/memory.h | 16 ++++++++++++++++ xen/include/xlat.lst | 1 + xen/include/xsm/xsm.h | 6 ++++++ xen/xsm/dummy.c | 6 ++++++ xen/xsm/flask/hooks.c | 6 ++++++ 8 files changed, 118 insertions(+), 0 deletions(-) diff --git a/xen/arch/ia64/xen/mm.c b/xen/arch/ia64/xen/mm.c index 694bf95..1b42e25 100644 --- a/xen/arch/ia64/xen/mm.c +++ b/xen/arch/ia64/xen/mm.c @@ -3447,6 +3447,40 @@ arch_memory_op(int op, XEN_GUEST_HANDLE(void) arg) break; } + case XENMEM_remove_from_physmap: + { + struct xen_remove_from_physmap xrfp; + unsigned long mfn; + struct domain *d; + + if ( copy_from_guest(&xrfp, arg, 1) ) + return -EFAULT; + + rc = rcu_lock_target_domain_by_id(xrfp.domid, &d); + if ( rc != 0 ) + return rc; + + if ( xsm_remove_from_physmap(current->domain, d) ) + { + rcu_unlock_domain(d); + return -EPERM; + } + + domain_lock(d); + + mfn = gmfn_to_mfn(d, xrfp.gpfn); + + if ( mfn_valid(mfn) ) + guest_physmap_remove_page(d, xrfp.gpfn, mfn, 0); + + domain_unlock(d); + + rcu_unlock_domain(d); + + break; + } + + case XENMEM_machine_memory_map: { struct xen_memory_map memmap; diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c index 52222dd..0b5332b 100644 --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -4871,6 +4871,41 @@ long arch_memory_op(int op, XEN_GUEST_HANDLE(void) arg) return rc; } + case XENMEM_remove_from_physmap: + { + struct xen_remove_from_physmap xrfp; + unsigned long mfn; + struct domain *d; + + if ( copy_from_guest(&xrfp, arg, 1) ) + return -EFAULT; + + rc = rcu_lock_target_domain_by_id(xrfp.domid, &d); + if ( rc != 0 ) + return rc; + + if ( xsm_remove_from_physmap(current->domain, d) ) + { + rcu_unlock_domain(d); + return -EPERM; + } + + domain_lock(d); + + mfn = get_gfn_untyped(d, xrfp.gpfn); + + if ( mfn_valid(mfn) ) + guest_physmap_remove_page(d, xrfp.gpfn, mfn, PAGE_ORDER_4K); + + put_gfn(d, xrfp.gpfn); + + domain_unlock(d); + + rcu_unlock_domain(d); + + break; + } + case XENMEM_set_memory_map: { struct xen_foreign_memory_map fmap; diff --git a/xen/arch/x86/x86_64/compat/mm.c b/xen/arch/x86/x86_64/compat/mm.c index bea94fe..dde5430 100644 --- a/xen/arch/x86/x86_64/compat/mm.c +++ b/xen/arch/x86/x86_64/compat/mm.c @@ -82,6 +82,20 @@ int compat_arch_memory_op(int op, XEN_GUEST_HANDLE(void) arg) break; } + case XENMEM_remove_from_physmap: + { + struct compat_remove_from_physmap cmp; + struct xen_remove_from_physmap *nat = (void *)COMPAT_ARG_XLAT_VIRT_BASE; + + if ( copy_from_guest(&cmp, arg, 1) ) + return -EFAULT; + + XLAT_remove_from_physmap(nat, &cmp); + rc = arch_memory_op(op, guest_handle_from_ptr(nat, void)); + + break; + } + case XENMEM_set_memory_map: { struct compat_foreign_memory_map cmp; diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h index c5b78a8..308deff 100644 --- a/xen/include/public/memory.h +++ b/xen/include/public/memory.h @@ -229,6 +229,22 @@ struct xen_add_to_physmap { typedef struct xen_add_to_physmap xen_add_to_physmap_t; DEFINE_XEN_GUEST_HANDLE(xen_add_to_physmap_t); +/* + * Unmaps the page appearing at a particular GPFN from the specified guest's + * pseudophysical address space. + * arg == addr of xen_remove_from_physmap_t. + */ +#define XENMEM_remove_from_physmap 15 +struct xen_remove_from_physmap { + /* Which domain to change the mapping for. */ + domid_t domid; + + /* GPFN of the current mapping of the page. */ + xen_pfn_t gpfn; +}; +typedef struct xen_remove_from_physmap xen_remove_from_physmap_t; +DEFINE_XEN_GUEST_HANDLE(xen_remove_from_physmap_t); + /*** REMOVED ***/ /*#define XENMEM_translate_gpfn_list 8*/ diff --git a/xen/include/xlat.lst b/xen/include/xlat.lst index 3d92175..9e67259 100644 --- a/xen/include/xlat.lst +++ b/xen/include/xlat.lst @@ -54,6 +54,7 @@ ! kexec_image kexec.h ! kexec_range kexec.h ! add_to_physmap memory.h +! remove_from_physmap memory.h ! foreign_memory_map memory.h ! memory_exchange memory.h ! memory_map memory.h diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h index df6cec2..566c808 100644 --- a/xen/include/xsm/xsm.h +++ b/xen/include/xsm/xsm.h @@ -169,6 +169,7 @@ struct xsm_operations { int (*update_va_mapping) (struct domain *d, struct domain *f, l1_pgentry_t pte); int (*add_to_physmap) (struct domain *d1, struct domain *d2); + int (*remove_from_physmap) (struct domain *d1, struct domain *d2); int (*sendtrigger) (struct domain *d); int (*bind_pt_irq) (struct domain *d, struct xen_domctl_bind_pt_irq *bind); int (*unbind_pt_irq) (struct domain *d); @@ -738,6 +739,11 @@ static inline int xsm_add_to_physmap(struct domain *d1, struct domain *d2) return xsm_call(add_to_physmap(d1, d2)); } +static inline int xsm_remove_from_physmap(struct domain *d1, struct domain *d2) +{ + return xsm_call(remove_from_physmap(d1, d2)); +} + static inline int xsm_sendtrigger(struct domain *d) { return xsm_call(sendtrigger(d)); diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c index 4bbfbff..65daa4e 100644 --- a/xen/xsm/dummy.c +++ b/xen/xsm/dummy.c @@ -529,6 +529,11 @@ static int dummy_add_to_physmap (struct domain *d1, struct domain *d2) return 0; } +static int dummy_remove_from_physmap (struct domain *d1, struct domain *d2) +{ + return 0; +} + static int dummy_sendtrigger (struct domain *d) { return 0; @@ -690,6 +695,7 @@ void xsm_fixup_ops (struct xsm_operations *ops) set_to_dummy_if_null(ops, mmu_machphys_update); set_to_dummy_if_null(ops, update_va_mapping); set_to_dummy_if_null(ops, add_to_physmap); + set_to_dummy_if_null(ops, remove_from_physmap); set_to_dummy_if_null(ops, sendtrigger); set_to_dummy_if_null(ops, bind_pt_irq); set_to_dummy_if_null(ops, pin_mem_cacheattr); diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c index 0d35767..a2020a9 100644 --- a/xen/xsm/flask/hooks.c +++ b/xen/xsm/flask/hooks.c @@ -1283,6 +1283,11 @@ static int flask_add_to_physmap(struct domain *d1, struct domain *d2) return domain_has_perm(d1, d2, SECCLASS_MMU, MMU__PHYSMAP); } +static int flask_remove_from_physmap(struct domain *d1, struct domain *d2) +{ + return domain_has_perm(d1, d2, SECCLASS_MMU, MMU__PHYSMAP); +} + static int flask_sendtrigger(struct domain *d) { return domain_has_perm(current->domain, d, SECCLASS_DOMAIN, DOMAIN__TRIGGER); @@ -1550,6 +1555,7 @@ static struct xsm_operations flask_ops = { .mmu_machphys_update = flask_mmu_machphys_update, .update_va_mapping = flask_update_va_mapping, .add_to_physmap = flask_add_to_physmap, + .remove_from_physmap = flask_remove_from_physmap, .sendtrigger = flask_sendtrigger, .get_device_group = flask_get_device_group, .test_assign_device = flask_test_assign_device, -- 1.7.7.5 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 01/18] xen: reinstate previously unused XENMEM_remove_from_physmap hypercall 2012-01-11 17:21 ` [PATCH 01/18] xen: reinstate previously unused XENMEM_remove_from_physmap hypercall Daniel De Graaf @ 2012-01-12 8:22 ` Jan Beulich 0 siblings, 0 replies; 11+ messages in thread From: Jan Beulich @ 2012-01-12 8:22 UTC (permalink / raw) To: Daniel De Graaf; +Cc: Alex Zeffertt, xen-devel >>> On 11.01.12 at 18:21, Daniel De Graaf <dgdegra@tycho.nsa.gov> wrote: > --- a/xen/include/xlat.lst > +++ b/xen/include/xlat.lst > @@ -54,6 +54,7 @@ > ! kexec_image kexec.h > ! kexec_range kexec.h > ! add_to_physmap memory.h > +! remove_from_physmap memory.h > ! foreign_memory_map memory.h > ! memory_exchange memory.h > ! memory_map memory.h Alphabetically, please. Jan ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 00/18] Xenstore stub domain 2012-01-11 17:21 [RFC PATCH 0/18] Xenstore stub domain Daniel De Graaf 2012-01-11 17:21 ` [PATCH 01/18] xen: reinstate previously unused XENMEM_remove_from_physmap hypercall Daniel De Graaf @ 2012-01-12 23:35 ` Daniel De Graaf 2012-01-12 23:35 ` [PATCH 01/18] xen: reinstate previously unused XENMEM_remove_from_physmap hypercall Daniel De Graaf 1 sibling, 1 reply; 11+ messages in thread From: Daniel De Graaf @ 2012-01-12 23:35 UTC (permalink / raw) To: xen-devel Changes from v1: - set_virq_handler implemented in libxc - added custom domain builder for xenstored - xenstore/console domain IDs now pulled from xenstore - migration support when using split xenstored (untested, should work) - slightly less intrusive NO_SOCKETS xenstored patch (still has many ifdefs to avoid pulling in socket headers or symbols) - virq handlers removed from dying domain when clearing event channels - dummy XSM module restricts getdomaininfo similar to no-XSM case - formatting/type fixups - partial ioctl compatibility with legacy IOCTL_XENBUS_ALLOC (the new device path uses 'B' not 'X' as the base so the ioctl number does not match; otherwise, should be compatible). To start xenstored, run: tools/xenstore/init-xenstore-domain stubdom/mini-os-x86_64-xenstore/mini-os 20 system_u:system_r:domU_t This will populate the xenstore domid key /tool/xenstore/domid ---- [PATCH 01/18] xen: reinstate previously unused hypercall [PATCH 02/18] xen: allow global VIRQ handlers to be delegated to [PATCH 03/18] xen: use XSM instead of IS_PRIV for getdomaininfo [PATCH 04/18] xen: Preserve reserved grant entries when switching [PATCH 05/18] tools/libxl: pull xenstore/console domids from [PATCH 06/18] lib{xc,xl}: Seed grant tables with xenstore and [PATCH 07/18] mini-os: avoid crash if no console is provided [PATCH 08/18] mini-os: avoid crash if no xenstore is provided [PATCH 09/18] mini-os: remove per-fd evtchn limit [PATCH 10/18] xenstored: use grant references instead of [PATCH 11/18] xenstored: add NO_SOCKETS compilation option [PATCH 12/18] xenstored support for in-memory rather than FS based [PATCH 13/18] xenstored: support running in minios stubdom [PATCH 14/18] xenstored: always use xc_gnttab_munmap in stubdom [PATCH 15/18] xenstored: add --event parameter for bootstrapping Removed old #16; shared page is no longer used to pass event. [PATCH 16/18] xenstored: use domain_is_unprivileged instead of [PATCH 17/18] xenstored: add --priv-domid parameter [PATCH 18/18] xenstored: Add stub domain builder ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 01/18] xen: reinstate previously unused XENMEM_remove_from_physmap hypercall 2012-01-12 23:35 ` [PATCH v2 00/18] Xenstore stub domain Daniel De Graaf @ 2012-01-12 23:35 ` Daniel De Graaf 2012-01-13 7:56 ` Jan Beulich 2012-01-18 10:36 ` Ian Campbell 0 siblings, 2 replies; 11+ messages in thread From: Daniel De Graaf @ 2012-01-12 23:35 UTC (permalink / raw) To: xen-devel; +Cc: Daniel De Graaf, Alex Zeffertt From: Alex Zeffertt <alex.zeffertt@eu.citrix.com> This patch reinstates the XENMEM_remove_from_physmap hypercall which was removed in 19041:ee62aaafff46 because it was not used. However, is now needed in order to support xenstored stub domains. The xenstored stub domain is not priviliged like dom0 and so cannot unilaterally map the xenbus page of other guests into it's address space. Therefore, before creating a domU the domain builder needs to seed its grant table with a grant ref allowing the xenstored stub domain to access the new domU's xenbus page. At present domU's do not start with their grant table mapped. Instead it gets mapped when the guest requests a grant table from the hypervisor. In order to seed the grant table, the domain builder first needs to map it into dom0 address space. But the hypercall to do this requires a gpfn (guest pfn), which is an mfn for PV guest, but a pfn for HVM guests. Therfore, in order to seed the grant table of an HVM guest, dom0 needs to *temporarily* map it into the guest's "physical" address space. Hence the need to reinstate the XENMEM_remove_from_physmap hypercall. Signed-off-by: Alex Zeffertt <alex.zeffertt@eu.citrix.com> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> --- xen/arch/ia64/xen/mm.c | 34 ++++++++++++++++++++++++++++++++++ xen/arch/x86/mm.c | 35 +++++++++++++++++++++++++++++++++++ xen/arch/x86/x86_64/compat/mm.c | 14 ++++++++++++++ xen/include/public/memory.h | 16 ++++++++++++++++ xen/include/xlat.lst | 1 + xen/include/xsm/xsm.h | 6 ++++++ xen/xsm/dummy.c | 6 ++++++ xen/xsm/flask/hooks.c | 6 ++++++ 8 files changed, 118 insertions(+), 0 deletions(-) diff --git a/xen/arch/ia64/xen/mm.c b/xen/arch/ia64/xen/mm.c index 84f6a61..a40f96a 100644 --- a/xen/arch/ia64/xen/mm.c +++ b/xen/arch/ia64/xen/mm.c @@ -3448,6 +3448,40 @@ arch_memory_op(int op, XEN_GUEST_HANDLE(void) arg) break; } + case XENMEM_remove_from_physmap: + { + struct xen_remove_from_physmap xrfp; + unsigned long mfn; + struct domain *d; + + if ( copy_from_guest(&xrfp, arg, 1) ) + return -EFAULT; + + rc = rcu_lock_target_domain_by_id(xrfp.domid, &d); + if ( rc != 0 ) + return rc; + + if ( xsm_remove_from_physmap(current->domain, d) ) + { + rcu_unlock_domain(d); + return -EPERM; + } + + domain_lock(d); + + mfn = gmfn_to_mfn(d, xrfp.gpfn); + + if ( mfn_valid(mfn) ) + guest_physmap_remove_page(d, xrfp.gpfn, mfn, 0); + + domain_unlock(d); + + rcu_unlock_domain(d); + + break; + } + + case XENMEM_machine_memory_map: { struct xen_memory_map memmap; diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c index 1f996e0..39cc3c0 100644 --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -4871,6 +4871,41 @@ long arch_memory_op(int op, XEN_GUEST_HANDLE(void) arg) return rc; } + case XENMEM_remove_from_physmap: + { + struct xen_remove_from_physmap xrfp; + unsigned long mfn; + struct domain *d; + + if ( copy_from_guest(&xrfp, arg, 1) ) + return -EFAULT; + + rc = rcu_lock_target_domain_by_id(xrfp.domid, &d); + if ( rc != 0 ) + return rc; + + if ( xsm_remove_from_physmap(current->domain, d) ) + { + rcu_unlock_domain(d); + return -EPERM; + } + + domain_lock(d); + + mfn = get_gfn_untyped(d, xrfp.gpfn); + + if ( mfn_valid(mfn) ) + guest_physmap_remove_page(d, xrfp.gpfn, mfn, PAGE_ORDER_4K); + + put_gfn(d, xrfp.gpfn); + + domain_unlock(d); + + rcu_unlock_domain(d); + + break; + } + case XENMEM_set_memory_map: { struct xen_foreign_memory_map fmap; diff --git a/xen/arch/x86/x86_64/compat/mm.c b/xen/arch/x86/x86_64/compat/mm.c index bea94fe..dde5430 100644 --- a/xen/arch/x86/x86_64/compat/mm.c +++ b/xen/arch/x86/x86_64/compat/mm.c @@ -82,6 +82,20 @@ int compat_arch_memory_op(int op, XEN_GUEST_HANDLE(void) arg) break; } + case XENMEM_remove_from_physmap: + { + struct compat_remove_from_physmap cmp; + struct xen_remove_from_physmap *nat = (void *)COMPAT_ARG_XLAT_VIRT_BASE; + + if ( copy_from_guest(&cmp, arg, 1) ) + return -EFAULT; + + XLAT_remove_from_physmap(nat, &cmp); + rc = arch_memory_op(op, guest_handle_from_ptr(nat, void)); + + break; + } + case XENMEM_set_memory_map: { struct compat_foreign_memory_map cmp; diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h index c5b78a8..308deff 100644 --- a/xen/include/public/memory.h +++ b/xen/include/public/memory.h @@ -229,6 +229,22 @@ struct xen_add_to_physmap { typedef struct xen_add_to_physmap xen_add_to_physmap_t; DEFINE_XEN_GUEST_HANDLE(xen_add_to_physmap_t); +/* + * Unmaps the page appearing at a particular GPFN from the specified guest's + * pseudophysical address space. + * arg == addr of xen_remove_from_physmap_t. + */ +#define XENMEM_remove_from_physmap 15 +struct xen_remove_from_physmap { + /* Which domain to change the mapping for. */ + domid_t domid; + + /* GPFN of the current mapping of the page. */ + xen_pfn_t gpfn; +}; +typedef struct xen_remove_from_physmap xen_remove_from_physmap_t; +DEFINE_XEN_GUEST_HANDLE(xen_remove_from_physmap_t); + /*** REMOVED ***/ /*#define XENMEM_translate_gpfn_list 8*/ diff --git a/xen/include/xlat.lst b/xen/include/xlat.lst index 3d92175..ee1772c 100644 --- a/xen/include/xlat.lst +++ b/xen/include/xlat.lst @@ -81,6 +81,7 @@ ! processor_power platform.h ? processor_px platform.h ! psd_package platform.h +! remove_from_physmap memory.h ? xenpf_pcpuinfo platform.h ? xenpf_pcpu_version platform.h ! sched_poll sched.h diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h index df6cec2..566c808 100644 --- a/xen/include/xsm/xsm.h +++ b/xen/include/xsm/xsm.h @@ -169,6 +169,7 @@ struct xsm_operations { int (*update_va_mapping) (struct domain *d, struct domain *f, l1_pgentry_t pte); int (*add_to_physmap) (struct domain *d1, struct domain *d2); + int (*remove_from_physmap) (struct domain *d1, struct domain *d2); int (*sendtrigger) (struct domain *d); int (*bind_pt_irq) (struct domain *d, struct xen_domctl_bind_pt_irq *bind); int (*unbind_pt_irq) (struct domain *d); @@ -738,6 +739,11 @@ static inline int xsm_add_to_physmap(struct domain *d1, struct domain *d2) return xsm_call(add_to_physmap(d1, d2)); } +static inline int xsm_remove_from_physmap(struct domain *d1, struct domain *d2) +{ + return xsm_call(remove_from_physmap(d1, d2)); +} + static inline int xsm_sendtrigger(struct domain *d) { return xsm_call(sendtrigger(d)); diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c index 4bbfbff..65daa4e 100644 --- a/xen/xsm/dummy.c +++ b/xen/xsm/dummy.c @@ -529,6 +529,11 @@ static int dummy_add_to_physmap (struct domain *d1, struct domain *d2) return 0; } +static int dummy_remove_from_physmap (struct domain *d1, struct domain *d2) +{ + return 0; +} + static int dummy_sendtrigger (struct domain *d) { return 0; @@ -690,6 +695,7 @@ void xsm_fixup_ops (struct xsm_operations *ops) set_to_dummy_if_null(ops, mmu_machphys_update); set_to_dummy_if_null(ops, update_va_mapping); set_to_dummy_if_null(ops, add_to_physmap); + set_to_dummy_if_null(ops, remove_from_physmap); set_to_dummy_if_null(ops, sendtrigger); set_to_dummy_if_null(ops, bind_pt_irq); set_to_dummy_if_null(ops, pin_mem_cacheattr); diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c index 0d35767..a2020a9 100644 --- a/xen/xsm/flask/hooks.c +++ b/xen/xsm/flask/hooks.c @@ -1283,6 +1283,11 @@ static int flask_add_to_physmap(struct domain *d1, struct domain *d2) return domain_has_perm(d1, d2, SECCLASS_MMU, MMU__PHYSMAP); } +static int flask_remove_from_physmap(struct domain *d1, struct domain *d2) +{ + return domain_has_perm(d1, d2, SECCLASS_MMU, MMU__PHYSMAP); +} + static int flask_sendtrigger(struct domain *d) { return domain_has_perm(current->domain, d, SECCLASS_DOMAIN, DOMAIN__TRIGGER); @@ -1550,6 +1555,7 @@ static struct xsm_operations flask_ops = { .mmu_machphys_update = flask_mmu_machphys_update, .update_va_mapping = flask_update_va_mapping, .add_to_physmap = flask_add_to_physmap, + .remove_from_physmap = flask_remove_from_physmap, .sendtrigger = flask_sendtrigger, .get_device_group = flask_get_device_group, .test_assign_device = flask_test_assign_device, -- 1.7.7.5 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 01/18] xen: reinstate previously unused XENMEM_remove_from_physmap hypercall 2012-01-12 23:35 ` [PATCH 01/18] xen: reinstate previously unused XENMEM_remove_from_physmap hypercall Daniel De Graaf @ 2012-01-13 7:56 ` Jan Beulich 2012-01-18 10:36 ` Ian Campbell 1 sibling, 0 replies; 11+ messages in thread From: Jan Beulich @ 2012-01-13 7:56 UTC (permalink / raw) To: Daniel De Graaf; +Cc: Alex Zeffertt, xen-devel >>> On 13.01.12 at 00:35, Daniel De Graaf <dgdegra@tycho.nsa.gov> wrote: > --- a/xen/arch/ia64/xen/mm.c > +++ b/xen/arch/ia64/xen/mm.c > @@ -3448,6 +3448,40 @@ arch_memory_op(int op, XEN_GUEST_HANDLE(void) arg) > break; > } > > + case XENMEM_remove_from_physmap: > + { > + struct xen_remove_from_physmap xrfp; > + unsigned long mfn; > + struct domain *d; > + > + if ( copy_from_guest(&xrfp, arg, 1) ) > + return -EFAULT; > + > + rc = rcu_lock_target_domain_by_id(xrfp.domid, &d); > + if ( rc != 0 ) > + return rc; > + > + if ( xsm_remove_from_physmap(current->domain, d) ) > + { > + rcu_unlock_domain(d); > + return -EPERM; > + } > + > + domain_lock(d); > + > + mfn = gmfn_to_mfn(d, xrfp.gpfn); > + > + if ( mfn_valid(mfn) ) > + guest_physmap_remove_page(d, xrfp.gpfn, mfn, 0); Here and in the other similar paths - is it really to be considered 'success' if !mfn_valid(mfn)? Further, given that the code looks all the same between ia64 and x86 - is this really arch-specific (the get_gfn_untyped() and put_gfn() used in x86 should really be stubbed out for ia64 anyway)? > + > + domain_unlock(d); > + > + rcu_unlock_domain(d); > + > + break; > + } > + > + > case XENMEM_machine_memory_map: > { > struct xen_memory_map memmap; > --- a/xen/include/xlat.lst > +++ b/xen/include/xlat.lst > @@ -81,6 +81,7 @@ > ! processor_power platform.h > ? processor_px platform.h > ! psd_package platform.h > +! remove_from_physmap memory.h Sorry, but this is not how I meant my comment on v1 to be interpreted - I thought that by looking at the rest of the file it would go without saying that the sorting is by header name (alphabetically), then by type name (again alphabetically). (I realize there are other cases where sorting is not fully correct, but as I'm striving to adjust this via secondary changes, I'd like to avoid getting the situation worse with new additions.) > ? xenpf_pcpuinfo platform.h > ? xenpf_pcpu_version platform.h > ! sched_poll sched.h Jan ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 01/18] xen: reinstate previously unused XENMEM_remove_from_physmap hypercall 2012-01-12 23:35 ` [PATCH 01/18] xen: reinstate previously unused XENMEM_remove_from_physmap hypercall Daniel De Graaf 2012-01-13 7:56 ` Jan Beulich @ 2012-01-18 10:36 ` Ian Campbell 2012-01-18 14:56 ` Daniel De Graaf 1 sibling, 1 reply; 11+ messages in thread From: Ian Campbell @ 2012-01-18 10:36 UTC (permalink / raw) To: Daniel De Graaf; +Cc: Alex Zeffertt, xen-devel On Thu, 2012-01-12 at 23:35 +0000, Daniel De Graaf wrote: > From: Alex Zeffertt <alex.zeffertt@eu.citrix.com> > > This patch reinstates the XENMEM_remove_from_physmap hypercall > which was removed in 19041:ee62aaafff46 because it was not used. > > However, is now needed in order to support xenstored stub domains. > The xenstored stub domain is not priviliged like dom0 and so cannot > unilaterally map the xenbus page of other guests into it's address > space. Therefore, before creating a domU the domain builder needs to > seed its grant table with a grant ref allowing the xenstored stub > domain to access the new domU's xenbus page. > > At present domU's do not start with their grant table mapped. > Instead it gets mapped when the guest requests a grant table from > the hypervisor. > > In order to seed the grant table, the domain builder first needs to > map it into dom0 address space. But the hypercall to do this > requires a gpfn (guest pfn), which is an mfn for PV guest, but a pfn > for HVM guests. Therfore, in order to seed the grant table of an > HVM guest, dom0 needs to *temporarily* map it into the guest's > "physical" address space. > > Hence the need to reinstate the XENMEM_remove_from_physmap hypercall. > > Signed-off-by: Alex Zeffertt <alex.zeffertt@eu.citrix.com> > Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> Acked-by: Ian Campbell <ian.campbell@citrix.com> (modulo Jan's comment about ordering in xlat.lst) BTW, since Alex and Diego have subsequently left Citrix you could take my Acked-by's in this series as Signed-of-by on behalf of Citrix. I've no idea if that's necessary though, I expect not. Ian. > --- > xen/arch/ia64/xen/mm.c | 34 ++++++++++++++++++++++++++++++++++ > xen/arch/x86/mm.c | 35 +++++++++++++++++++++++++++++++++++ > xen/arch/x86/x86_64/compat/mm.c | 14 ++++++++++++++ > xen/include/public/memory.h | 16 ++++++++++++++++ > xen/include/xlat.lst | 1 + > xen/include/xsm/xsm.h | 6 ++++++ > xen/xsm/dummy.c | 6 ++++++ > xen/xsm/flask/hooks.c | 6 ++++++ > 8 files changed, 118 insertions(+), 0 deletions(-) > > diff --git a/xen/arch/ia64/xen/mm.c b/xen/arch/ia64/xen/mm.c > index 84f6a61..a40f96a 100644 > --- a/xen/arch/ia64/xen/mm.c > +++ b/xen/arch/ia64/xen/mm.c > @@ -3448,6 +3448,40 @@ arch_memory_op(int op, XEN_GUEST_HANDLE(void) arg) > break; > } > > + case XENMEM_remove_from_physmap: > + { > + struct xen_remove_from_physmap xrfp; > + unsigned long mfn; > + struct domain *d; > + > + if ( copy_from_guest(&xrfp, arg, 1) ) > + return -EFAULT; > + > + rc = rcu_lock_target_domain_by_id(xrfp.domid, &d); > + if ( rc != 0 ) > + return rc; > + > + if ( xsm_remove_from_physmap(current->domain, d) ) > + { > + rcu_unlock_domain(d); > + return -EPERM; > + } > + > + domain_lock(d); > + > + mfn = gmfn_to_mfn(d, xrfp.gpfn); > + > + if ( mfn_valid(mfn) ) > + guest_physmap_remove_page(d, xrfp.gpfn, mfn, 0); > + > + domain_unlock(d); > + > + rcu_unlock_domain(d); > + > + break; > + } > + > + > case XENMEM_machine_memory_map: > { > struct xen_memory_map memmap; > diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c > index 1f996e0..39cc3c0 100644 > --- a/xen/arch/x86/mm.c > +++ b/xen/arch/x86/mm.c > @@ -4871,6 +4871,41 @@ long arch_memory_op(int op, XEN_GUEST_HANDLE(void) arg) > return rc; > } > > + case XENMEM_remove_from_physmap: > + { > + struct xen_remove_from_physmap xrfp; > + unsigned long mfn; > + struct domain *d; > + > + if ( copy_from_guest(&xrfp, arg, 1) ) > + return -EFAULT; > + > + rc = rcu_lock_target_domain_by_id(xrfp.domid, &d); > + if ( rc != 0 ) > + return rc; > + > + if ( xsm_remove_from_physmap(current->domain, d) ) > + { > + rcu_unlock_domain(d); > + return -EPERM; > + } > + > + domain_lock(d); > + > + mfn = get_gfn_untyped(d, xrfp.gpfn); > + > + if ( mfn_valid(mfn) ) > + guest_physmap_remove_page(d, xrfp.gpfn, mfn, PAGE_ORDER_4K); > + > + put_gfn(d, xrfp.gpfn); > + > + domain_unlock(d); > + > + rcu_unlock_domain(d); > + > + break; > + } > + > case XENMEM_set_memory_map: > { > struct xen_foreign_memory_map fmap; > diff --git a/xen/arch/x86/x86_64/compat/mm.c b/xen/arch/x86/x86_64/compat/mm.c > index bea94fe..dde5430 100644 > --- a/xen/arch/x86/x86_64/compat/mm.c > +++ b/xen/arch/x86/x86_64/compat/mm.c > @@ -82,6 +82,20 @@ int compat_arch_memory_op(int op, XEN_GUEST_HANDLE(void) arg) > break; > } > > + case XENMEM_remove_from_physmap: > + { > + struct compat_remove_from_physmap cmp; > + struct xen_remove_from_physmap *nat = (void *)COMPAT_ARG_XLAT_VIRT_BASE; > + > + if ( copy_from_guest(&cmp, arg, 1) ) > + return -EFAULT; > + > + XLAT_remove_from_physmap(nat, &cmp); > + rc = arch_memory_op(op, guest_handle_from_ptr(nat, void)); > + > + break; > + } > + > case XENMEM_set_memory_map: > { > struct compat_foreign_memory_map cmp; > diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h > index c5b78a8..308deff 100644 > --- a/xen/include/public/memory.h > +++ b/xen/include/public/memory.h > @@ -229,6 +229,22 @@ struct xen_add_to_physmap { > typedef struct xen_add_to_physmap xen_add_to_physmap_t; > DEFINE_XEN_GUEST_HANDLE(xen_add_to_physmap_t); > > +/* > + * Unmaps the page appearing at a particular GPFN from the specified guest's > + * pseudophysical address space. > + * arg == addr of xen_remove_from_physmap_t. > + */ > +#define XENMEM_remove_from_physmap 15 > +struct xen_remove_from_physmap { > + /* Which domain to change the mapping for. */ > + domid_t domid; > + > + /* GPFN of the current mapping of the page. */ > + xen_pfn_t gpfn; > +}; > +typedef struct xen_remove_from_physmap xen_remove_from_physmap_t; > +DEFINE_XEN_GUEST_HANDLE(xen_remove_from_physmap_t); > + > /*** REMOVED ***/ > /*#define XENMEM_translate_gpfn_list 8*/ > > diff --git a/xen/include/xlat.lst b/xen/include/xlat.lst > index 3d92175..ee1772c 100644 > --- a/xen/include/xlat.lst > +++ b/xen/include/xlat.lst > @@ -81,6 +81,7 @@ > ! processor_power platform.h > ? processor_px platform.h > ! psd_package platform.h > +! remove_from_physmap memory.h > ? xenpf_pcpuinfo platform.h > ? xenpf_pcpu_version platform.h > ! sched_poll sched.h > diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h > index df6cec2..566c808 100644 > --- a/xen/include/xsm/xsm.h > +++ b/xen/include/xsm/xsm.h > @@ -169,6 +169,7 @@ struct xsm_operations { > int (*update_va_mapping) (struct domain *d, struct domain *f, > l1_pgentry_t pte); > int (*add_to_physmap) (struct domain *d1, struct domain *d2); > + int (*remove_from_physmap) (struct domain *d1, struct domain *d2); > int (*sendtrigger) (struct domain *d); > int (*bind_pt_irq) (struct domain *d, struct xen_domctl_bind_pt_irq *bind); > int (*unbind_pt_irq) (struct domain *d); > @@ -738,6 +739,11 @@ static inline int xsm_add_to_physmap(struct domain *d1, struct domain *d2) > return xsm_call(add_to_physmap(d1, d2)); > } > > +static inline int xsm_remove_from_physmap(struct domain *d1, struct domain *d2) > +{ > + return xsm_call(remove_from_physmap(d1, d2)); > +} > + > static inline int xsm_sendtrigger(struct domain *d) > { > return xsm_call(sendtrigger(d)); > diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c > index 4bbfbff..65daa4e 100644 > --- a/xen/xsm/dummy.c > +++ b/xen/xsm/dummy.c > @@ -529,6 +529,11 @@ static int dummy_add_to_physmap (struct domain *d1, struct domain *d2) > return 0; > } > > +static int dummy_remove_from_physmap (struct domain *d1, struct domain *d2) > +{ > + return 0; > +} > + > static int dummy_sendtrigger (struct domain *d) > { > return 0; > @@ -690,6 +695,7 @@ void xsm_fixup_ops (struct xsm_operations *ops) > set_to_dummy_if_null(ops, mmu_machphys_update); > set_to_dummy_if_null(ops, update_va_mapping); > set_to_dummy_if_null(ops, add_to_physmap); > + set_to_dummy_if_null(ops, remove_from_physmap); > set_to_dummy_if_null(ops, sendtrigger); > set_to_dummy_if_null(ops, bind_pt_irq); > set_to_dummy_if_null(ops, pin_mem_cacheattr); > diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c > index 0d35767..a2020a9 100644 > --- a/xen/xsm/flask/hooks.c > +++ b/xen/xsm/flask/hooks.c > @@ -1283,6 +1283,11 @@ static int flask_add_to_physmap(struct domain *d1, struct domain *d2) > return domain_has_perm(d1, d2, SECCLASS_MMU, MMU__PHYSMAP); > } > > +static int flask_remove_from_physmap(struct domain *d1, struct domain *d2) > +{ > + return domain_has_perm(d1, d2, SECCLASS_MMU, MMU__PHYSMAP); > +} > + > static int flask_sendtrigger(struct domain *d) > { > return domain_has_perm(current->domain, d, SECCLASS_DOMAIN, DOMAIN__TRIGGER); > @@ -1550,6 +1555,7 @@ static struct xsm_operations flask_ops = { > .mmu_machphys_update = flask_mmu_machphys_update, > .update_va_mapping = flask_update_va_mapping, > .add_to_physmap = flask_add_to_physmap, > + .remove_from_physmap = flask_remove_from_physmap, > .sendtrigger = flask_sendtrigger, > .get_device_group = flask_get_device_group, > .test_assign_device = flask_test_assign_device, ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 01/18] xen: reinstate previously unused XENMEM_remove_from_physmap hypercall 2012-01-18 10:36 ` Ian Campbell @ 2012-01-18 14:56 ` Daniel De Graaf 2012-01-18 16:06 ` Ian Campbell 0 siblings, 1 reply; 11+ messages in thread From: Daniel De Graaf @ 2012-01-18 14:56 UTC (permalink / raw) To: Ian Campbell; +Cc: Alex Zeffertt, xen-devel On 01/18/2012 05:36 AM, Ian Campbell wrote: > On Thu, 2012-01-12 at 23:35 +0000, Daniel De Graaf wrote: >> From: Alex Zeffertt <alex.zeffertt@eu.citrix.com> >> >> This patch reinstates the XENMEM_remove_from_physmap hypercall >> which was removed in 19041:ee62aaafff46 because it was not used. >> >> However, is now needed in order to support xenstored stub domains. >> The xenstored stub domain is not priviliged like dom0 and so cannot >> unilaterally map the xenbus page of other guests into it's address >> space. Therefore, before creating a domU the domain builder needs to >> seed its grant table with a grant ref allowing the xenstored stub >> domain to access the new domU's xenbus page. >> >> At present domU's do not start with their grant table mapped. >> Instead it gets mapped when the guest requests a grant table from >> the hypervisor. >> >> In order to seed the grant table, the domain builder first needs to >> map it into dom0 address space. But the hypercall to do this >> requires a gpfn (guest pfn), which is an mfn for PV guest, but a pfn >> for HVM guests. Therfore, in order to seed the grant table of an >> HVM guest, dom0 needs to *temporarily* map it into the guest's >> "physical" address space. >> >> Hence the need to reinstate the XENMEM_remove_from_physmap hypercall. >> >> Signed-off-by: Alex Zeffertt <alex.zeffertt@eu.citrix.com> >> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> > > Acked-by: Ian Campbell <ian.campbell@citrix.com> (modulo Jan's comment > about ordering in xlat.lst) > > BTW, since Alex and Diego have subsequently left Citrix you could take > my Acked-by's in this series as Signed-of-by on behalf of Citrix. I've > no idea if that's necessary though, I expect not. > > Ian. > I'm not an expert in this area, but this is how I read it: the portion of the path authored by Alex/Diego was already signed-off when they were posted, so since the current patches are derived works from them the sign-off may need to stay in order to allow me to sign off because I cannot claim copyright on all of the content. Assuming Citrix actually owns the copyright on the patches, your Ack may suffice to replace the sign-off for this purpose. I guess my real question here would be: should the sign-off from Alex and Diego remain on these patches in addition to your Ack? ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 01/18] xen: reinstate previously unused XENMEM_remove_from_physmap hypercall 2012-01-18 14:56 ` Daniel De Graaf @ 2012-01-18 16:06 ` Ian Campbell 2012-01-18 19:07 ` Daniel De Graaf 0 siblings, 1 reply; 11+ messages in thread From: Ian Campbell @ 2012-01-18 16:06 UTC (permalink / raw) To: Daniel De Graaf; +Cc: Alex Zeffertt, xen-devel On Wed, 2012-01-18 at 14:56 +0000, Daniel De Graaf wrote: > On 01/18/2012 05:36 AM, Ian Campbell wrote: > > On Thu, 2012-01-12 at 23:35 +0000, Daniel De Graaf wrote: > >> From: Alex Zeffertt <alex.zeffertt@eu.citrix.com> > >> > >> This patch reinstates the XENMEM_remove_from_physmap hypercall > >> which was removed in 19041:ee62aaafff46 because it was not used. > >> > >> However, is now needed in order to support xenstored stub domains. > >> The xenstored stub domain is not priviliged like dom0 and so cannot > >> unilaterally map the xenbus page of other guests into it's address > >> space. Therefore, before creating a domU the domain builder needs to > >> seed its grant table with a grant ref allowing the xenstored stub > >> domain to access the new domU's xenbus page. > >> > >> At present domU's do not start with their grant table mapped. > >> Instead it gets mapped when the guest requests a grant table from > >> the hypervisor. > >> > >> In order to seed the grant table, the domain builder first needs to > >> map it into dom0 address space. But the hypercall to do this > >> requires a gpfn (guest pfn), which is an mfn for PV guest, but a pfn > >> for HVM guests. Therfore, in order to seed the grant table of an > >> HVM guest, dom0 needs to *temporarily* map it into the guest's > >> "physical" address space. > >> > >> Hence the need to reinstate the XENMEM_remove_from_physmap hypercall. > >> > >> Signed-off-by: Alex Zeffertt <alex.zeffertt@eu.citrix.com> > >> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> > > > > Acked-by: Ian Campbell <ian.campbell@citrix.com> (modulo Jan's comment > > about ordering in xlat.lst) > > > > BTW, since Alex and Diego have subsequently left Citrix you could take > > my Acked-by's in this series as Signed-of-by on behalf of Citrix. I've > > no idea if that's necessary though, I expect not. > > > > Ian. > > > > I'm not an expert in this area, Me neither. > but this is how I read it: the portion of > the path authored by Alex/Diego was already signed-off when they were posted, > so since the current patches are derived works from them the sign-off may > need to stay in order to allow me to sign off because I cannot claim copyright > on all of the content. Assuming Citrix actually owns the copyright on the > patches, your Ack may suffice to replace the sign-off for this purpose. I don't think an Ack conveys the same meaning (WRT the DCO) as a Signed-off-by. > I guess my real question here would be: should the sign-off from Alex and > Diego remain on these patches in addition to your Ack? I would suggest you keep any signed-off-by they provided and augment it with my ack. I think I saw one or two which said "Originally-by" instead of "Signed-of-by", I guess those were either missing a Signed-off-by in the first place or have been heavily modified? Ian. > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 01/18] xen: reinstate previously unused XENMEM_remove_from_physmap hypercall 2012-01-18 16:06 ` Ian Campbell @ 2012-01-18 19:07 ` Daniel De Graaf 2012-01-19 10:32 ` Ian Campbell 0 siblings, 1 reply; 11+ messages in thread From: Daniel De Graaf @ 2012-01-18 19:07 UTC (permalink / raw) To: Ian Campbell; +Cc: xen-devel On 01/18/2012 11:06 AM, Ian Campbell wrote: > On Wed, 2012-01-18 at 14:56 +0000, Daniel De Graaf wrote: >> On 01/18/2012 05:36 AM, Ian Campbell wrote: >>> On Thu, 2012-01-12 at 23:35 +0000, Daniel De Graaf wrote: >>>> From: Alex Zeffertt <alex.zeffertt@eu.citrix.com> >>>> >>>> This patch reinstates the XENMEM_remove_from_physmap hypercall >>>> which was removed in 19041:ee62aaafff46 because it was not used. >>>> >>>> However, is now needed in order to support xenstored stub domains. >>>> The xenstored stub domain is not priviliged like dom0 and so cannot >>>> unilaterally map the xenbus page of other guests into it's address >>>> space. Therefore, before creating a domU the domain builder needs to >>>> seed its grant table with a grant ref allowing the xenstored stub >>>> domain to access the new domU's xenbus page. >>>> >>>> At present domU's do not start with their grant table mapped. >>>> Instead it gets mapped when the guest requests a grant table from >>>> the hypervisor. >>>> >>>> In order to seed the grant table, the domain builder first needs to >>>> map it into dom0 address space. But the hypercall to do this >>>> requires a gpfn (guest pfn), which is an mfn for PV guest, but a pfn >>>> for HVM guests. Therfore, in order to seed the grant table of an >>>> HVM guest, dom0 needs to *temporarily* map it into the guest's >>>> "physical" address space. >>>> >>>> Hence the need to reinstate the XENMEM_remove_from_physmap hypercall. >>>> >>>> Signed-off-by: Alex Zeffertt <alex.zeffertt@eu.citrix.com> >>>> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> >>> >>> Acked-by: Ian Campbell <ian.campbell@citrix.com> (modulo Jan's comment >>> about ordering in xlat.lst) >>> >>> BTW, since Alex and Diego have subsequently left Citrix you could take >>> my Acked-by's in this series as Signed-of-by on behalf of Citrix. I've >>> no idea if that's necessary though, I expect not. >>> >>> Ian. >>> >> >> I'm not an expert in this area, > > Me neither. > >> but this is how I read it: the portion of >> the path authored by Alex/Diego was already signed-off when they were posted, >> so since the current patches are derived works from them the sign-off may >> need to stay in order to allow me to sign off because I cannot claim copyright >> on all of the content. Assuming Citrix actually owns the copyright on the >> patches, your Ack may suffice to replace the sign-off for this purpose. > > I don't think an Ack conveys the same meaning (WRT the DCO) as a > Signed-off-by. > >> I guess my real question here would be: should the sign-off from Alex and >> Diego remain on these patches in addition to your Ack? > > I would suggest you keep any signed-off-by they provided and augment it > with my ack. > > I think I saw one or two which said "Originally-by" instead of > "Signed-of-by", I guess those were either missing a Signed-off-by in the > first place or have been heavily modified? > > Ian. > I originally replaced all the signed-off-by lines with originally-by and missed one when converting back. When looking at the Linux version of the DCO, it implies (lower down when talking about subsystem maintainers) that if I make changes I need to drop the sign-off and claim clause (b) unless the original author is around to sign-off on the changed patch, or if it is trivial and I note this above my sign-off (not applicable here). This makes me lean toward changing back to "Originally-by" or similar tags. I did keep the From tags for those patches that I did not mostly rewrite, which I assume will be recognized when importing patches. -- Daniel De Graaf National Security Agency ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 01/18] xen: reinstate previously unused XENMEM_remove_from_physmap hypercall 2012-01-18 19:07 ` Daniel De Graaf @ 2012-01-19 10:32 ` Ian Campbell 0 siblings, 0 replies; 11+ messages in thread From: Ian Campbell @ 2012-01-19 10:32 UTC (permalink / raw) To: Daniel De Graaf; +Cc: xen-devel On Wed, 2012-01-18 at 19:07 +0000, Daniel De Graaf wrote: > On 01/18/2012 11:06 AM, Ian Campbell wrote: > > On Wed, 2012-01-18 at 14:56 +0000, Daniel De Graaf wrote: > >> On 01/18/2012 05:36 AM, Ian Campbell wrote: > >>> On Thu, 2012-01-12 at 23:35 +0000, Daniel De Graaf wrote: > >>>> From: Alex Zeffertt <alex.zeffertt@eu.citrix.com> > >>>> > >>>> This patch reinstates the XENMEM_remove_from_physmap hypercall > >>>> which was removed in 19041:ee62aaafff46 because it was not used. > >>>> > >>>> However, is now needed in order to support xenstored stub domains. > >>>> The xenstored stub domain is not priviliged like dom0 and so cannot > >>>> unilaterally map the xenbus page of other guests into it's address > >>>> space. Therefore, before creating a domU the domain builder needs to > >>>> seed its grant table with a grant ref allowing the xenstored stub > >>>> domain to access the new domU's xenbus page. > >>>> > >>>> At present domU's do not start with their grant table mapped. > >>>> Instead it gets mapped when the guest requests a grant table from > >>>> the hypervisor. > >>>> > >>>> In order to seed the grant table, the domain builder first needs to > >>>> map it into dom0 address space. But the hypercall to do this > >>>> requires a gpfn (guest pfn), which is an mfn for PV guest, but a pfn > >>>> for HVM guests. Therfore, in order to seed the grant table of an > >>>> HVM guest, dom0 needs to *temporarily* map it into the guest's > >>>> "physical" address space. > >>>> > >>>> Hence the need to reinstate the XENMEM_remove_from_physmap hypercall. > >>>> > >>>> Signed-off-by: Alex Zeffertt <alex.zeffertt@eu.citrix.com> > >>>> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov> > >>> > >>> Acked-by: Ian Campbell <ian.campbell@citrix.com> (modulo Jan's comment > >>> about ordering in xlat.lst) > >>> > >>> BTW, since Alex and Diego have subsequently left Citrix you could take > >>> my Acked-by's in this series as Signed-of-by on behalf of Citrix. I've > >>> no idea if that's necessary though, I expect not. > >>> > >>> Ian. > >>> > >> > >> I'm not an expert in this area, > > > > Me neither. > > > >> but this is how I read it: the portion of > >> the path authored by Alex/Diego was already signed-off when they were posted, > >> so since the current patches are derived works from them the sign-off may > >> need to stay in order to allow me to sign off because I cannot claim copyright > >> on all of the content. Assuming Citrix actually owns the copyright on the > >> patches, your Ack may suffice to replace the sign-off for this purpose. > > > > I don't think an Ack conveys the same meaning (WRT the DCO) as a > > Signed-off-by. > > > >> I guess my real question here would be: should the sign-off from Alex and > >> Diego remain on these patches in addition to your Ack? > > > > I would suggest you keep any signed-off-by they provided and augment it > > with my ack. > > > > I think I saw one or two which said "Originally-by" instead of > > "Signed-of-by", I guess those were either missing a Signed-off-by in the > > first place or have been heavily modified? > > > > Ian. > > > > I originally replaced all the signed-off-by lines with originally-by and > missed one when converting back. When looking at the Linux version of the > DCO, it implies (lower down when talking about subsystem maintainers) that > if I make changes I need to drop the sign-off and claim clause (b) unless > the original author is around to sign-off on the changed patch, or if it is > trivial and I note this above my sign-off (not applicable here). This makes > me lean toward changing back to "Originally-by" or similar tags. I did keep > the From tags for those patches that I did not mostly rewrite, which I assume > will be recognized when importing patches. The DCO itself isn't terribly specific about what to do with an existing Signed-off-by if you modify the patch. Common practice appears to be to include both the original and your own and to note what you have changed unless you have done a wholesale rewrite in which case it is "Based-on"/"Originally-by"/etc + your own S-o-b. Ian. > ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2012-01-19 10:32 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <mailman.745.1326901231.1471.xen-devel@lists.xensource.com> 2012-01-18 15:47 ` [PATCH 01/18] xen: reinstate previously unused XENMEM_remove_from_physmap hypercall Andres Lagar-Cavilla [not found] <mailman.700.1326883173.1471.xen-devel@lists.xensource.com> 2012-01-18 15:40 ` Andres Lagar-Cavilla 2012-01-11 17:21 [RFC PATCH 0/18] Xenstore stub domain Daniel De Graaf 2012-01-11 17:21 ` [PATCH 01/18] xen: reinstate previously unused XENMEM_remove_from_physmap hypercall Daniel De Graaf 2012-01-12 8:22 ` Jan Beulich 2012-01-12 23:35 ` [PATCH v2 00/18] Xenstore stub domain Daniel De Graaf 2012-01-12 23:35 ` [PATCH 01/18] xen: reinstate previously unused XENMEM_remove_from_physmap hypercall Daniel De Graaf 2012-01-13 7:56 ` Jan Beulich 2012-01-18 10:36 ` Ian Campbell 2012-01-18 14:56 ` Daniel De Graaf 2012-01-18 16:06 ` Ian Campbell 2012-01-18 19:07 ` Daniel De Graaf 2012-01-19 10:32 ` Ian Campbell
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.