* IOREQ server on Arm @ 2018-09-25 22:39 Julien Grall 2018-09-26 8:08 ` Jan Beulich 0 siblings, 1 reply; 23+ messages in thread From: Julien Grall @ 2018-09-25 22:39 UTC (permalink / raw) To: Paul Durrant, xen-devel Cc: Andrew Cooper, Stefano Stabellini, Jan Beulich, Roger Pau Monné Hi Paul, I am looking at porting the IOREQ server infrastructure on Arm. I didn't need much modification to make it run for Arm. Although, the implementation could be simplified over the x86 implementation. I noticed some issue while trying to implement the hypercall XENMEM_acquire_resource. Per my understanding, all the page mapped via that hypercall will use the type p2m_mapping_foreign. This will result to trigger the ASSERT(fdom != dom) in get_page_from_gfn (asm-arm/p2m.h) because the IOREQ page has been allocated to the emulator domain and mapped to it. AFAICT x86 has the same assert in p2m_get_page_from_gfn(...). IHMO, the ASSERT makes sense because you are only meant to map page belonging to other domain with that type. So I am wondering whether IOREQ server running in PVH Dom0 has been tested? What would be the best course of action to fix the issue? Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: IOREQ server on Arm 2018-09-25 22:39 IOREQ server on Arm Julien Grall @ 2018-09-26 8:08 ` Jan Beulich 2018-09-26 9:14 ` Paul Durrant 2018-09-26 10:41 ` Julien Grall 0 siblings, 2 replies; 23+ messages in thread From: Jan Beulich @ 2018-09-26 8:08 UTC (permalink / raw) To: Julien Grall, Paul Durrant Cc: Andrew Cooper, Stefano Stabellini, xen-devel, Roger Pau Monne >>> On 26.09.18 at 00:39, <julien.grall@arm.com> wrote: > Hi Paul, > > I am looking at porting the IOREQ server infrastructure on Arm. I didn't > need much modification to make it run for Arm. Although, the > implementation could be simplified over the x86 implementation. > > I noticed some issue while trying to implement the hypercall > XENMEM_acquire_resource. Per my understanding, all the page mapped via > that hypercall will use the type p2m_mapping_foreign. > > This will result to trigger the ASSERT(fdom != dom) in get_page_from_gfn > (asm-arm/p2m.h) because the IOREQ page has been allocated to the > emulator domain and mapped to it. AFAICT x86 has the same assert in > p2m_get_page_from_gfn(...). > > IHMO, the ASSERT makes sense because you are only meant to map page > belonging to other domain with that type. > > So I am wondering whether IOREQ server running in PVH Dom0 has been > tested? What would be the best course of action to fix the issue? I think the p2m type needs to be chosen based on XENMEM_rsrc_acq_caller_owned. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: IOREQ server on Arm 2018-09-26 8:08 ` Jan Beulich @ 2018-09-26 9:14 ` Paul Durrant 2018-09-26 10:32 ` Julien Grall 2018-09-26 10:41 ` Julien Grall 1 sibling, 1 reply; 23+ messages in thread From: Paul Durrant @ 2018-09-26 9:14 UTC (permalink / raw) To: 'Jan Beulich', Julien Grall Cc: Andrew Cooper, Stefano Stabellini, xen-devel, Roger Pau Monne > -----Original Message----- > From: Jan Beulich [mailto:JBeulich@suse.com] > Sent: 26 September 2018 09:09 > To: Julien Grall <julien.grall@arm.com>; Paul Durrant > <Paul.Durrant@citrix.com> > Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Roger Pau Monne > <roger.pau@citrix.com>; Stefano Stabellini <sstabellini@kernel.org>; xen- > devel <xen-devel@lists.xenproject.org> > Subject: Re: IOREQ server on Arm > > >>> On 26.09.18 at 00:39, <julien.grall@arm.com> wrote: > > Hi Paul, > > > > I am looking at porting the IOREQ server infrastructure on Arm. I didn't > > need much modification to make it run for Arm. Although, the > > implementation could be simplified over the x86 implementation. > > > > I noticed some issue while trying to implement the hypercall > > XENMEM_acquire_resource. Per my understanding, all the page mapped via > > that hypercall will use the type p2m_mapping_foreign. > > > > This will result to trigger the ASSERT(fdom != dom) in get_page_from_gfn > > (asm-arm/p2m.h) because the IOREQ page has been allocated to the > > emulator domain and mapped to it. AFAICT x86 has the same assert in > > p2m_get_page_from_gfn(...). > > > > IHMO, the ASSERT makes sense because you are only meant to map page > > belonging to other domain with that type. > > > > So I am wondering whether IOREQ server running in PVH Dom0 has been > > tested? What would be the best course of action to fix the issue? > > I think the p2m type needs to be chosen based on > XENMEM_rsrc_acq_caller_owned. > Yes, that's correct. There is a FIXME clause in acquire_resource so that that only caller owned resources can be mapped by HVM/PVH domains. Thus the new call can be used for IOREQ server pages, but not grant frames. Paul > Jan > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: IOREQ server on Arm 2018-09-26 9:14 ` Paul Durrant @ 2018-09-26 10:32 ` Julien Grall 2018-09-26 10:38 ` Andrew Cooper ` (2 more replies) 0 siblings, 3 replies; 23+ messages in thread From: Julien Grall @ 2018-09-26 10:32 UTC (permalink / raw) To: Paul Durrant, 'Jan Beulich' Cc: Andrew Cooper, Stefano Stabellini, xen-devel, Roger Pau Monne Hi, On 09/26/2018 10:14 AM, Paul Durrant wrote: >> -----Original Message----- >> From: Jan Beulich [mailto:JBeulich@suse.com] >> Sent: 26 September 2018 09:09 >> To: Julien Grall <julien.grall@arm.com>; Paul Durrant >> <Paul.Durrant@citrix.com> >> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Roger Pau Monne >> <roger.pau@citrix.com>; Stefano Stabellini <sstabellini@kernel.org>; xen- >> devel <xen-devel@lists.xenproject.org> >> Subject: Re: IOREQ server on Arm >> >>>>> On 26.09.18 at 00:39, <julien.grall@arm.com> wrote: >>> Hi Paul, >>> >>> I am looking at porting the IOREQ server infrastructure on Arm. I didn't >>> need much modification to make it run for Arm. Although, the >>> implementation could be simplified over the x86 implementation. >>> >>> I noticed some issue while trying to implement the hypercall >>> XENMEM_acquire_resource. Per my understanding, all the page mapped via >>> that hypercall will use the type p2m_mapping_foreign. >>> >>> This will result to trigger the ASSERT(fdom != dom) in get_page_from_gfn >>> (asm-arm/p2m.h) because the IOREQ page has been allocated to the >>> emulator domain and mapped to it. AFAICT x86 has the same assert in >>> p2m_get_page_from_gfn(...). >>> >>> IHMO, the ASSERT makes sense because you are only meant to map page >>> belonging to other domain with that type. >>> >>> So I am wondering whether IOREQ server running in PVH Dom0 has been >>> tested? What would be the best course of action to fix the issue? >> >> I think the p2m type needs to be chosen based on >> XENMEM_rsrc_acq_caller_owned. >> > > Yes, that's correct. There is a FIXME clause in acquire_resource so that that only caller owned resources can be mapped by HVM/PVH domains. Thus the new call can be used for IOREQ server pages, but not grant frames. I don't understand your last sentence. IOREQ is caller owned and therefore should work. As I said, I don't have any problem with mapping the resource. The problem is when unmapping it because the region is set with p2m_mapping_foreign. You will reach the ASSERT(fdom != p2m->domain) in p2m_get_page_from_gfn. Regardless the reference problem (we support it on Arm). Can you explain how this is working on PVH Dom0 today? Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: IOREQ server on Arm 2018-09-26 10:32 ` Julien Grall @ 2018-09-26 10:38 ` Andrew Cooper 2018-09-26 10:49 ` Paul Durrant 2018-10-01 15:56 ` Roger Pau Monné 2 siblings, 0 replies; 23+ messages in thread From: Andrew Cooper @ 2018-09-26 10:38 UTC (permalink / raw) To: Julien Grall, Paul Durrant, 'Jan Beulich' Cc: xen-devel, Stefano Stabellini, Roger Pau Monne On 26/09/18 11:32, Julien Grall wrote: > Hi, > > On 09/26/2018 10:14 AM, Paul Durrant wrote: >>> -----Original Message----- >>> From: Jan Beulich [mailto:JBeulich@suse.com] >>> Sent: 26 September 2018 09:09 >>> To: Julien Grall <julien.grall@arm.com>; Paul Durrant >>> <Paul.Durrant@citrix.com> >>> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Roger Pau Monne >>> <roger.pau@citrix.com>; Stefano Stabellini <sstabellini@kernel.org>; >>> xen- >>> devel <xen-devel@lists.xenproject.org> >>> Subject: Re: IOREQ server on Arm >>> >>>>>> On 26.09.18 at 00:39, <julien.grall@arm.com> wrote: >>>> Hi Paul, >>>> >>>> I am looking at porting the IOREQ server infrastructure on Arm. I >>>> didn't >>>> need much modification to make it run for Arm. Although, the >>>> implementation could be simplified over the x86 implementation. >>>> >>>> I noticed some issue while trying to implement the hypercall >>>> XENMEM_acquire_resource. Per my understanding, all the page mapped via >>>> that hypercall will use the type p2m_mapping_foreign. >>>> >>>> This will result to trigger the ASSERT(fdom != dom) in >>>> get_page_from_gfn >>>> (asm-arm/p2m.h) because the IOREQ page has been allocated to the >>>> emulator domain and mapped to it. AFAICT x86 has the same assert in >>>> p2m_get_page_from_gfn(...). >>>> >>>> IHMO, the ASSERT makes sense because you are only meant to map page >>>> belonging to other domain with that type. >>>> >>>> So I am wondering whether IOREQ server running in PVH Dom0 has been >>>> tested? What would be the best course of action to fix the issue? >>> >>> I think the p2m type needs to be chosen based on >>> XENMEM_rsrc_acq_caller_owned. >>> >> >> Yes, that's correct. There is a FIXME clause in acquire_resource so >> that that only caller owned resources can be mapped by HVM/PVH >> domains. Thus the new call can be used for IOREQ server pages, but >> not grant frames. > > I don't understand your last sentence. IOREQ is caller owned and > therefore should work. > > As I said, I don't have any problem with mapping the resource. The > problem is when unmapping it because the region is set with > p2m_mapping_foreign. You will reach the ASSERT(fdom != p2m->domain) in > p2m_get_page_from_gfn. > > Regardless the reference problem (we support it on Arm). Can you > explain how this is working on PVH Dom0 today? I doubt anyone has tried using it with a PVH Dom0. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: IOREQ server on Arm 2018-09-26 10:32 ` Julien Grall 2018-09-26 10:38 ` Andrew Cooper @ 2018-09-26 10:49 ` Paul Durrant 2018-10-01 15:56 ` Roger Pau Monné 2 siblings, 0 replies; 23+ messages in thread From: Paul Durrant @ 2018-09-26 10:49 UTC (permalink / raw) To: 'Julien Grall', 'Jan Beulich' Cc: Andrew Cooper, Stefano Stabellini, xen-devel, Roger Pau Monne > -----Original Message----- > From: Julien Grall [mailto:julien.grall@arm.com] > Sent: 26 September 2018 11:33 > To: Paul Durrant <Paul.Durrant@citrix.com>; 'Jan Beulich' > <JBeulich@suse.com> > Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Roger Pau Monne > <roger.pau@citrix.com>; Stefano Stabellini <sstabellini@kernel.org>; xen- > devel <xen-devel@lists.xenproject.org> > Subject: Re: IOREQ server on Arm > > Hi, > > On 09/26/2018 10:14 AM, Paul Durrant wrote: > >> -----Original Message----- > >> From: Jan Beulich [mailto:JBeulich@suse.com] > >> Sent: 26 September 2018 09:09 > >> To: Julien Grall <julien.grall@arm.com>; Paul Durrant > >> <Paul.Durrant@citrix.com> > >> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Roger Pau Monne > >> <roger.pau@citrix.com>; Stefano Stabellini <sstabellini@kernel.org>; > xen- > >> devel <xen-devel@lists.xenproject.org> > >> Subject: Re: IOREQ server on Arm > >> > >>>>> On 26.09.18 at 00:39, <julien.grall@arm.com> wrote: > >>> Hi Paul, > >>> > >>> I am looking at porting the IOREQ server infrastructure on Arm. I > didn't > >>> need much modification to make it run for Arm. Although, the > >>> implementation could be simplified over the x86 implementation. > >>> > >>> I noticed some issue while trying to implement the hypercall > >>> XENMEM_acquire_resource. Per my understanding, all the page mapped via > >>> that hypercall will use the type p2m_mapping_foreign. > >>> > >>> This will result to trigger the ASSERT(fdom != dom) in > get_page_from_gfn > >>> (asm-arm/p2m.h) because the IOREQ page has been allocated to the > >>> emulator domain and mapped to it. AFAICT x86 has the same assert in > >>> p2m_get_page_from_gfn(...). > >>> > >>> IHMO, the ASSERT makes sense because you are only meant to map page > >>> belonging to other domain with that type. > >>> > >>> So I am wondering whether IOREQ server running in PVH Dom0 has been > >>> tested? What would be the best course of action to fix the issue? > >> > >> I think the p2m type needs to be chosen based on > >> XENMEM_rsrc_acq_caller_owned. > >> > > > > Yes, that's correct. There is a FIXME clause in acquire_resource so that > that only caller owned resources can be mapped by HVM/PVH domains. Thus > the new call can be used for IOREQ server pages, but not grant frames. > > I don't understand your last sentence. IOREQ is caller owned and > therefore should work. Yes, I was just saying that it is currently the only resource that should work. > > As I said, I don't have any problem with mapping the resource. The > problem is when unmapping it because the region is set with > p2m_mapping_foreign. You will reach the ASSERT(fdom != p2m->domain) in > p2m_get_page_from_gfn. > > Regardless the reference problem (we support it on Arm). Can you explain > how this is working on PVH Dom0 today? > I never tested with a PVH dom0, but I did run tests with an HVM domU (and a Xen with the privilege checks hacked out). I guess I didn't re-test with HVM after the switch to caller-owned (which came quite late in review) and so missed this problem. Sorry about that. Paul > Cheers, > > -- > Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: IOREQ server on Arm 2018-09-26 10:32 ` Julien Grall 2018-09-26 10:38 ` Andrew Cooper 2018-09-26 10:49 ` Paul Durrant @ 2018-10-01 15:56 ` Roger Pau Monné 2 siblings, 0 replies; 23+ messages in thread From: Roger Pau Monné @ 2018-10-01 15:56 UTC (permalink / raw) To: Julien Grall Cc: Andrew Cooper, Paul Durrant, Stefano Stabellini, 'Jan Beulich', xen-devel On Wed, Sep 26, 2018 at 11:32:38AM +0100, Julien Grall wrote: > Hi, > > On 09/26/2018 10:14 AM, Paul Durrant wrote: > > > -----Original Message----- > > > From: Jan Beulich [mailto:JBeulich@suse.com] > > > Sent: 26 September 2018 09:09 > > > To: Julien Grall <julien.grall@arm.com>; Paul Durrant > > > <Paul.Durrant@citrix.com> > > > Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Roger Pau Monne > > > <roger.pau@citrix.com>; Stefano Stabellini <sstabellini@kernel.org>; xen- > > > devel <xen-devel@lists.xenproject.org> > > > Subject: Re: IOREQ server on Arm > > > > > > > > > On 26.09.18 at 00:39, <julien.grall@arm.com> wrote: > > > > Hi Paul, > > > > > > > > I am looking at porting the IOREQ server infrastructure on Arm. I didn't > > > > need much modification to make it run for Arm. Although, the > > > > implementation could be simplified over the x86 implementation. > > > > > > > > I noticed some issue while trying to implement the hypercall > > > > XENMEM_acquire_resource. Per my understanding, all the page mapped via > > > > that hypercall will use the type p2m_mapping_foreign. > > > > > > > > This will result to trigger the ASSERT(fdom != dom) in get_page_from_gfn > > > > (asm-arm/p2m.h) because the IOREQ page has been allocated to the > > > > emulator domain and mapped to it. AFAICT x86 has the same assert in > > > > p2m_get_page_from_gfn(...). > > > > > > > > IHMO, the ASSERT makes sense because you are only meant to map page > > > > belonging to other domain with that type. > > > > > > > > So I am wondering whether IOREQ server running in PVH Dom0 has been > > > > tested? What would be the best course of action to fix the issue? > > > > > > I think the p2m type needs to be chosen based on > > > XENMEM_rsrc_acq_caller_owned. > > > > > > > Yes, that's correct. There is a FIXME clause in acquire_resource so that that only caller owned resources can be mapped by HVM/PVH domains. Thus the new call can be used for IOREQ server pages, but not grant frames. > > I don't understand your last sentence. IOREQ is caller owned and therefore > should work. > > As I said, I don't have any problem with mapping the resource. The problem > is when unmapping it because the region is set with p2m_mapping_foreign. You > will reach the ASSERT(fdom != p2m->domain) in p2m_get_page_from_gfn. > > Regardless the reference problem (we support it on Arm). Can you explain how > this is working on PVH Dom0 today? IOREQs (QEMU) used to work on a PVH Dom0 in the past, it's been some time since I've launched a HVM guest now, but it definitely worked before, like it worked for PVHv1 Dom0. Roger. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: IOREQ server on Arm 2018-09-26 8:08 ` Jan Beulich 2018-09-26 9:14 ` Paul Durrant @ 2018-09-26 10:41 ` Julien Grall 2018-09-26 10:51 ` Paul Durrant 1 sibling, 1 reply; 23+ messages in thread From: Julien Grall @ 2018-09-26 10:41 UTC (permalink / raw) To: Jan Beulich, Paul Durrant Cc: Andrew Cooper, Stefano Stabellini, xen-devel, Roger Pau Monne Hi Jan, On 09/26/2018 09:08 AM, Jan Beulich wrote: >>>> On 26.09.18 at 00:39, <julien.grall@arm.com> wrote: >> Hi Paul, >> >> I am looking at porting the IOREQ server infrastructure on Arm. I didn't >> need much modification to make it run for Arm. Although, the >> implementation could be simplified over the x86 implementation. >> >> I noticed some issue while trying to implement the hypercall >> XENMEM_acquire_resource. Per my understanding, all the page mapped via >> that hypercall will use the type p2m_mapping_foreign. >> >> This will result to trigger the ASSERT(fdom != dom) in get_page_from_gfn >> (asm-arm/p2m.h) because the IOREQ page has been allocated to the >> emulator domain and mapped to it. AFAICT x86 has the same assert in >> p2m_get_page_from_gfn(...). >> >> IHMO, the ASSERT makes sense because you are only meant to map page >> belonging to other domain with that type. >> >> So I am wondering whether IOREQ server running in PVH Dom0 has been >> tested? What would be the best course of action to fix the issue? > > I think the p2m type needs to be chosen based on > XENMEM_rsrc_acq_caller_owned. I am thinking to introduce p2m_mapping_owned. Or do we have a p2m_type that we could re-use? Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: IOREQ server on Arm 2018-09-26 10:41 ` Julien Grall @ 2018-09-26 10:51 ` Paul Durrant 2018-09-26 10:58 ` Jan Beulich 2018-09-26 11:01 ` Julien Grall 0 siblings, 2 replies; 23+ messages in thread From: Paul Durrant @ 2018-09-26 10:51 UTC (permalink / raw) To: 'Julien Grall', Jan Beulich Cc: Andrew Cooper, Stefano Stabellini, xen-devel, Roger Pau Monne > -----Original Message----- > From: Julien Grall [mailto:julien.grall@arm.com] > Sent: 26 September 2018 11:41 > To: Jan Beulich <JBeulich@suse.com>; Paul Durrant > <Paul.Durrant@citrix.com> > Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Roger Pau Monne > <roger.pau@citrix.com>; Stefano Stabellini <sstabellini@kernel.org>; xen- > devel <xen-devel@lists.xenproject.org> > Subject: Re: IOREQ server on Arm > > Hi Jan, > > On 09/26/2018 09:08 AM, Jan Beulich wrote: > >>>> On 26.09.18 at 00:39, <julien.grall@arm.com> wrote: > >> Hi Paul, > >> > >> I am looking at porting the IOREQ server infrastructure on Arm. I > didn't > >> need much modification to make it run for Arm. Although, the > >> implementation could be simplified over the x86 implementation. > >> > >> I noticed some issue while trying to implement the hypercall > >> XENMEM_acquire_resource. Per my understanding, all the page mapped via > >> that hypercall will use the type p2m_mapping_foreign. > >> > >> This will result to trigger the ASSERT(fdom != dom) in > get_page_from_gfn > >> (asm-arm/p2m.h) because the IOREQ page has been allocated to the > >> emulator domain and mapped to it. AFAICT x86 has the same assert in > >> p2m_get_page_from_gfn(...). > >> > >> IHMO, the ASSERT makes sense because you are only meant to map page > >> belonging to other domain with that type. > >> > >> So I am wondering whether IOREQ server running in PVH Dom0 has been > >> tested? What would be the best course of action to fix the issue? > > > > I think the p2m type needs to be chosen based on > > XENMEM_rsrc_acq_caller_owned. > > I am thinking to introduce p2m_mapping_owned. Or do we have a p2m_type > that we could re-use? > I think we should be able to just use p2m_ram_rw if it is caller owned. Paul > Cheers, > > -- > Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: IOREQ server on Arm 2018-09-26 10:51 ` Paul Durrant @ 2018-09-26 10:58 ` Jan Beulich 2018-09-26 11:02 ` Paul Durrant 2018-09-26 11:01 ` Julien Grall 1 sibling, 1 reply; 23+ messages in thread From: Jan Beulich @ 2018-09-26 10:58 UTC (permalink / raw) To: Julien Grall, Paul Durrant Cc: Andrew Cooper, Stefano Stabellini, xen-devel, Roger Pau Monne >>> On 26.09.18 at 12:51, <Paul.Durrant@citrix.com> wrote: >> -----Original Message----- >> From: Julien Grall [mailto:julien.grall@arm.com] >> Sent: 26 September 2018 11:41 >> To: Jan Beulich <JBeulich@suse.com>; Paul Durrant >> <Paul.Durrant@citrix.com> >> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Roger Pau Monne >> <roger.pau@citrix.com>; Stefano Stabellini <sstabellini@kernel.org>; xen- >> devel <xen-devel@lists.xenproject.org> >> Subject: Re: IOREQ server on Arm >> >> Hi Jan, >> >> On 09/26/2018 09:08 AM, Jan Beulich wrote: >> >>>> On 26.09.18 at 00:39, <julien.grall@arm.com> wrote: >> >> Hi Paul, >> >> >> >> I am looking at porting the IOREQ server infrastructure on Arm. I >> didn't >> >> need much modification to make it run for Arm. Although, the >> >> implementation could be simplified over the x86 implementation. >> >> >> >> I noticed some issue while trying to implement the hypercall >> >> XENMEM_acquire_resource. Per my understanding, all the page mapped via >> >> that hypercall will use the type p2m_mapping_foreign. >> >> >> >> This will result to trigger the ASSERT(fdom != dom) in >> get_page_from_gfn >> >> (asm-arm/p2m.h) because the IOREQ page has been allocated to the >> >> emulator domain and mapped to it. AFAICT x86 has the same assert in >> >> p2m_get_page_from_gfn(...). >> >> >> >> IHMO, the ASSERT makes sense because you are only meant to map page >> >> belonging to other domain with that type. >> >> >> >> So I am wondering whether IOREQ server running in PVH Dom0 has been >> >> tested? What would be the best course of action to fix the issue? >> > >> > I think the p2m type needs to be chosen based on >> > XENMEM_rsrc_acq_caller_owned. >> >> I am thinking to introduce p2m_mapping_owned. Or do we have a p2m_type >> that we could re-use? >> > > I think we should be able to just use p2m_ram_rw if it is caller owned. Yes, that's what I too would have thought. If there ever was a resource which may only be mapped r/o, p2m_ram_ro should then be equally usable. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: IOREQ server on Arm 2018-09-26 10:58 ` Jan Beulich @ 2018-09-26 11:02 ` Paul Durrant 2018-09-26 11:57 ` Jan Beulich 0 siblings, 1 reply; 23+ messages in thread From: Paul Durrant @ 2018-09-26 11:02 UTC (permalink / raw) To: 'Jan Beulich', Julien Grall Cc: Andrew Cooper, Stefano Stabellini, xen-devel, Roger Pau Monne > -----Original Message----- > From: Jan Beulich [mailto:JBeulich@suse.com] > Sent: 26 September 2018 11:58 > To: Julien Grall <julien.grall@arm.com>; Paul Durrant > <Paul.Durrant@citrix.com> > Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Roger Pau Monne > <roger.pau@citrix.com>; Stefano Stabellini <sstabellini@kernel.org>; xen- > devel <xen-devel@lists.xenproject.org> > Subject: RE: IOREQ server on Arm > > >>> On 26.09.18 at 12:51, <Paul.Durrant@citrix.com> wrote: > >> -----Original Message----- > >> From: Julien Grall [mailto:julien.grall@arm.com] > >> Sent: 26 September 2018 11:41 > >> To: Jan Beulich <JBeulich@suse.com>; Paul Durrant > >> <Paul.Durrant@citrix.com> > >> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Roger Pau Monne > >> <roger.pau@citrix.com>; Stefano Stabellini <sstabellini@kernel.org>; > xen- > >> devel <xen-devel@lists.xenproject.org> > >> Subject: Re: IOREQ server on Arm > >> > >> Hi Jan, > >> > >> On 09/26/2018 09:08 AM, Jan Beulich wrote: > >> >>>> On 26.09.18 at 00:39, <julien.grall@arm.com> wrote: > >> >> Hi Paul, > >> >> > >> >> I am looking at porting the IOREQ server infrastructure on Arm. I > >> didn't > >> >> need much modification to make it run for Arm. Although, the > >> >> implementation could be simplified over the x86 implementation. > >> >> > >> >> I noticed some issue while trying to implement the hypercall > >> >> XENMEM_acquire_resource. Per my understanding, all the page mapped > via > >> >> that hypercall will use the type p2m_mapping_foreign. > >> >> > >> >> This will result to trigger the ASSERT(fdom != dom) in > >> get_page_from_gfn > >> >> (asm-arm/p2m.h) because the IOREQ page has been allocated to the > >> >> emulator domain and mapped to it. AFAICT x86 has the same assert in > >> >> p2m_get_page_from_gfn(...). > >> >> > >> >> IHMO, the ASSERT makes sense because you are only meant to map page > >> >> belonging to other domain with that type. > >> >> > >> >> So I am wondering whether IOREQ server running in PVH Dom0 has been > >> >> tested? What would be the best course of action to fix the issue? > >> > > >> > I think the p2m type needs to be chosen based on > >> > XENMEM_rsrc_acq_caller_owned. > >> > >> I am thinking to introduce p2m_mapping_owned. Or do we have a p2m_type > >> that we could re-use? > >> > > > > I think we should be able to just use p2m_ram_rw if it is caller owned. > > Yes, that's what I too would have thought. If there ever was a resource > which may only be mapped r/o, p2m_ram_ro should then be equally usable. > Yes, that's true. The only existent resources are read-write though so I guess another flag passed back to the caller could be used to indicate a read-only resource. I was thinking along the lines of a patch like this: diff --git a/xen/common/memory.c b/xen/common/memory.c index 996f94b103..82c18fa9ad 100644 --- a/xen/common/memory.c +++ b/xen/common/memory.c @@ -1105,8 +1105,11 @@ static int acquire_resource( for ( i = 0; !rc && i < xmar.nr_frames; i++ ) { - rc = set_foreign_p2m_entry(currd, gfn_list[i], - _mfn(mfn_list[i])); + rc = (xmar.flags & XENMEM_rsrc_acq_caller_owned) ? + guest_physmap_add_entry(currd, gfn_list[i], + _mfn(mfn_list[i]), 0, p2m_ram_rw) : + set_foreign_p2m_entry(currd, gfn_list[i], + _mfn(mfn_list[i])); /* rc should be -EIO for any iteration other than the first */ if ( rc && i ) rc = -EIO; But the guest_physmap_add_entry() is problematic as it will IOMMU map pages as well, which is probably not wanted. Paul > Jan > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: IOREQ server on Arm 2018-09-26 11:02 ` Paul Durrant @ 2018-09-26 11:57 ` Jan Beulich 2018-09-26 12:01 ` Paul Durrant 0 siblings, 1 reply; 23+ messages in thread From: Jan Beulich @ 2018-09-26 11:57 UTC (permalink / raw) To: Paul Durrant Cc: Andrew Cooper, Julien Grall, Stefano Stabellini, xen-devel, Roger Pau Monne >>> On 26.09.18 at 13:02, <Paul.Durrant@citrix.com> wrote: > --- a/xen/common/memory.c > +++ b/xen/common/memory.c > @@ -1105,8 +1105,11 @@ static int acquire_resource( > > for ( i = 0; !rc && i < xmar.nr_frames; i++ ) > { > - rc = set_foreign_p2m_entry(currd, gfn_list[i], > - _mfn(mfn_list[i])); > + rc = (xmar.flags & XENMEM_rsrc_acq_caller_owned) ? > + guest_physmap_add_entry(currd, gfn_list[i], > + _mfn(mfn_list[i]), 0, p2m_ram_rw) : > + set_foreign_p2m_entry(currd, gfn_list[i], > + _mfn(mfn_list[i])); > /* rc should be -EIO for any iteration other than the first */ > if ( rc && i ) > rc = -EIO; > > But the guest_physmap_add_entry() is problematic as it will IOMMU map pages > as well, which is probably not wanted. Yeah, I'd prefer if we avoided establishing IOMMU mappings here. How about transforming set_foreign_p2m_entry() into set_special_p2m_entry(), with a type passed in? Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: IOREQ server on Arm 2018-09-26 11:57 ` Jan Beulich @ 2018-09-26 12:01 ` Paul Durrant 2018-09-26 21:32 ` Julien Grall 0 siblings, 1 reply; 23+ messages in thread From: Paul Durrant @ 2018-09-26 12:01 UTC (permalink / raw) To: 'Jan Beulich' Cc: Andrew Cooper, Julien Grall, Stefano Stabellini, xen-devel, Roger Pau Monne > -----Original Message----- > From: Jan Beulich [mailto:JBeulich@suse.com] > Sent: 26 September 2018 12:57 > To: Paul Durrant <Paul.Durrant@citrix.com> > Cc: Julien Grall <julien.grall@arm.com>; Andrew Cooper > <Andrew.Cooper3@citrix.com>; Roger Pau Monne <roger.pau@citrix.com>; > Stefano Stabellini <sstabellini@kernel.org>; xen-devel <xen- > devel@lists.xenproject.org> > Subject: RE: IOREQ server on Arm > > >>> On 26.09.18 at 13:02, <Paul.Durrant@citrix.com> wrote: > > --- a/xen/common/memory.c > > +++ b/xen/common/memory.c > > @@ -1105,8 +1105,11 @@ static int acquire_resource( > > > > for ( i = 0; !rc && i < xmar.nr_frames; i++ ) > > { > > - rc = set_foreign_p2m_entry(currd, gfn_list[i], > > - _mfn(mfn_list[i])); > > + rc = (xmar.flags & XENMEM_rsrc_acq_caller_owned) ? > > + guest_physmap_add_entry(currd, gfn_list[i], > > + _mfn(mfn_list[i]), 0, > p2m_ram_rw) : > > + set_foreign_p2m_entry(currd, gfn_list[i], > > + _mfn(mfn_list[i])); > > /* rc should be -EIO for any iteration other than the first > */ > > if ( rc && i ) > > rc = -EIO; > > > > But the guest_physmap_add_entry() is problematic as it will IOMMU map > pages > > as well, which is probably not wanted. > > Yeah, I'd prefer if we avoided establishing IOMMU mappings here. > How about transforming set_foreign_p2m_entry() into > set_special_p2m_entry(), with a type passed in? > That sounds like it might work. Julien, do you want page types to distinguish caller-owned resources from normal RAM are you ok with p2m_ram_rw even though it could be subject of another domain's foreign map? Paul > Jan > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: IOREQ server on Arm 2018-09-26 12:01 ` Paul Durrant @ 2018-09-26 21:32 ` Julien Grall 2018-09-27 6:10 ` Jan Beulich 2018-09-27 8:38 ` Paul Durrant 0 siblings, 2 replies; 23+ messages in thread From: Julien Grall @ 2018-09-26 21:32 UTC (permalink / raw) To: Paul Durrant, 'Jan Beulich' Cc: Andrew Cooper, Stefano Stabellini, xen-devel, Roger Pau Monne Hi Paul, On 09/26/2018 01:01 PM, Paul Durrant wrote: >> -----Original Message----- >> From: Jan Beulich [mailto:JBeulich@suse.com] >> Sent: 26 September 2018 12:57 >> To: Paul Durrant <Paul.Durrant@citrix.com> >> Cc: Julien Grall <julien.grall@arm.com>; Andrew Cooper >> <Andrew.Cooper3@citrix.com>; Roger Pau Monne <roger.pau@citrix.com>; >> Stefano Stabellini <sstabellini@kernel.org>; xen-devel <xen- >> devel@lists.xenproject.org> >> Subject: RE: IOREQ server on Arm >> >>>>> On 26.09.18 at 13:02, <Paul.Durrant@citrix.com> wrote: >>> --- a/xen/common/memory.c >>> +++ b/xen/common/memory.c >>> @@ -1105,8 +1105,11 @@ static int acquire_resource( >>> >>> for ( i = 0; !rc && i < xmar.nr_frames; i++ ) >>> { >>> - rc = set_foreign_p2m_entry(currd, gfn_list[i], >>> - _mfn(mfn_list[i])); >>> + rc = (xmar.flags & XENMEM_rsrc_acq_caller_owned) ? >>> + guest_physmap_add_entry(currd, gfn_list[i], >>> + _mfn(mfn_list[i]), 0, >> p2m_ram_rw) : >>> + set_foreign_p2m_entry(currd, gfn_list[i], >>> + _mfn(mfn_list[i])); >>> /* rc should be -EIO for any iteration other than the first >> */ >>> if ( rc && i ) >>> rc = -EIO; >>> >>> But the guest_physmap_add_entry() is problematic as it will IOMMU map >> pages >>> as well, which is probably not wanted. >> >> Yeah, I'd prefer if we avoided establishing IOMMU mappings here. >> How about transforming set_foreign_p2m_entry() into >> set_special_p2m_entry(), with a type passed in? >> > > That sounds like it might work. > > Julien, do you want page types to distinguish caller-owned resources from normal RAM are you ok with p2m_ram_rw even though it could be subject of another domain's foreign map? Based on your previous e-mail, I would be fine with that on Arm. This brings me to the next question. Do you expect set_special_p2m_entry to take a reference on the page? If not, we may run into some troubles because AFAICT you can map twice the ioreq page in a guest but reference will only be taken on the allocation. However, the unmap path will always drop a reference when removing the page. This is because Xen at the moment, reference will not be taken on mapping but allocation (we assume a page could not be mapped twice in a guest). Foreign mapping on Arm are a bit special because we get a reference on mapping them and will drop it when the mapping disappear. So we would not have any problem there. Any thoughts? -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: IOREQ server on Arm 2018-09-26 21:32 ` Julien Grall @ 2018-09-27 6:10 ` Jan Beulich 2018-09-27 8:38 ` Paul Durrant 1 sibling, 0 replies; 23+ messages in thread From: Jan Beulich @ 2018-09-27 6:10 UTC (permalink / raw) To: Julien Grall Cc: Andrew Cooper, Paul Durrant, xen-devel, Stefano Stabellini, Roger Pau Monne >>> On 26.09.18 at 23:32, <julien.grall@arm.com> wrote: > This brings me to the next question. Do you expect set_special_p2m_entry > to take a reference on the page? > > If not, we may run into some troubles because AFAICT you can map twice > the ioreq page in a guest but reference will only be taken on the > allocation. > > However, the unmap path will always drop a reference when removing the > page. This is because Xen at the moment, reference will not be taken on > mapping but allocation (we assume a page could not be mapped twice in a > guest). > > Foreign mapping on Arm are a bit special because we get a reference on > mapping them and will drop it when the mapping disappear. So we would > not have any problem there. Well, at least on x86 no refcounting happens at all in the p2m layer. That's something we mean to fix eventually, but I'm unaware of any work into that direction. If the situation is different on ARM, then the behavior of set_special_p2m_entry() may need to differ between it and x86 for the time being. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: IOREQ server on Arm 2018-09-26 21:32 ` Julien Grall 2018-09-27 6:10 ` Jan Beulich @ 2018-09-27 8:38 ` Paul Durrant 2018-09-27 9:41 ` Julien Grall 1 sibling, 1 reply; 23+ messages in thread From: Paul Durrant @ 2018-09-27 8:38 UTC (permalink / raw) To: 'Julien Grall', 'Jan Beulich' Cc: Andrew Cooper, Stefano Stabellini, xen-devel, Roger Pau Monne > -----Original Message----- > From: Julien Grall [mailto:julien.grall@arm.com] > Sent: 26 September 2018 22:32 > To: Paul Durrant <Paul.Durrant@citrix.com>; 'Jan Beulich' > <JBeulich@suse.com> > Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Roger Pau Monne > <roger.pau@citrix.com>; Stefano Stabellini <sstabellini@kernel.org>; xen- > devel <xen-devel@lists.xenproject.org> > Subject: Re: IOREQ server on Arm > > Hi Paul, > > On 09/26/2018 01:01 PM, Paul Durrant wrote: > >> -----Original Message----- > >> From: Jan Beulich [mailto:JBeulich@suse.com] > >> Sent: 26 September 2018 12:57 > >> To: Paul Durrant <Paul.Durrant@citrix.com> > >> Cc: Julien Grall <julien.grall@arm.com>; Andrew Cooper > >> <Andrew.Cooper3@citrix.com>; Roger Pau Monne <roger.pau@citrix.com>; > >> Stefano Stabellini <sstabellini@kernel.org>; xen-devel <xen- > >> devel@lists.xenproject.org> > >> Subject: RE: IOREQ server on Arm > >> > >>>>> On 26.09.18 at 13:02, <Paul.Durrant@citrix.com> wrote: > >>> --- a/xen/common/memory.c > >>> +++ b/xen/common/memory.c > >>> @@ -1105,8 +1105,11 @@ static int acquire_resource( > >>> > >>> for ( i = 0; !rc && i < xmar.nr_frames; i++ ) > >>> { > >>> - rc = set_foreign_p2m_entry(currd, gfn_list[i], > >>> - _mfn(mfn_list[i])); > >>> + rc = (xmar.flags & XENMEM_rsrc_acq_caller_owned) ? > >>> + guest_physmap_add_entry(currd, gfn_list[i], > >>> + _mfn(mfn_list[i]), 0, > >> p2m_ram_rw) : > >>> + set_foreign_p2m_entry(currd, gfn_list[i], > >>> + _mfn(mfn_list[i])); > >>> /* rc should be -EIO for any iteration other than the > first > >> */ > >>> if ( rc && i ) > >>> rc = -EIO; > >>> > >>> But the guest_physmap_add_entry() is problematic as it will IOMMU map > >> pages > >>> as well, which is probably not wanted. > >> > >> Yeah, I'd prefer if we avoided establishing IOMMU mappings here. > >> How about transforming set_foreign_p2m_entry() into > >> set_special_p2m_entry(), with a type passed in? > >> > > > > That sounds like it might work. > > > > Julien, do you want page types to distinguish caller-owned resources > from normal RAM are you ok with p2m_ram_rw even though it could be subject > of another domain's foreign map? > > Based on your previous e-mail, I would be fine with that on Arm. > > This brings me to the next question. Do you expect set_special_p2m_entry > to take a reference on the page? > > If not, we may run into some troubles because AFAICT you can map twice > the ioreq page in a guest but reference will only be taken on the > allocation. > > However, the unmap path will always drop a reference when removing the > page. This is because Xen at the moment, reference will not be taken on > mapping but allocation (we assume a page could not be mapped twice in a > guest). > > Foreign mapping on Arm are a bit special because we get a reference on > mapping them and will drop it when the mapping disappear. So we would > not have any problem there. > > Any thoughts? Well, as Jan says, on x86 we don't reference count in the P2M so multiple mappings should not be an issue AFAICT. It sounds like resource mapping should be treated the same way as foreign mapping (albeit with a non-foreign domid) such that the reference acquisition occurs at map time. Paul > > -- > Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: IOREQ server on Arm 2018-09-27 8:38 ` Paul Durrant @ 2018-09-27 9:41 ` Julien Grall 2018-09-27 10:16 ` Paul Durrant 0 siblings, 1 reply; 23+ messages in thread From: Julien Grall @ 2018-09-27 9:41 UTC (permalink / raw) To: Paul Durrant, 'Jan Beulich' Cc: Andrew Cooper, Stefano Stabellini, xen-devel, Roger Pau Monne Hi Paul, On 09/27/2018 09:38 AM, Paul Durrant wrote: >> -----Original Message----- >> From: Julien Grall [mailto:julien.grall@arm.com] >> Sent: 26 September 2018 22:32 >> To: Paul Durrant <Paul.Durrant@citrix.com>; 'Jan Beulich' >> <JBeulich@suse.com> >> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Roger Pau Monne >> <roger.pau@citrix.com>; Stefano Stabellini <sstabellini@kernel.org>; xen- >> devel <xen-devel@lists.xenproject.org> >> Subject: Re: IOREQ server on Arm >> >> Hi Paul, >> >> On 09/26/2018 01:01 PM, Paul Durrant wrote: >>>> -----Original Message----- >>>> From: Jan Beulich [mailto:JBeulich@suse.com] >>>> Sent: 26 September 2018 12:57 >>>> To: Paul Durrant <Paul.Durrant@citrix.com> >>>> Cc: Julien Grall <julien.grall@arm.com>; Andrew Cooper >>>> <Andrew.Cooper3@citrix.com>; Roger Pau Monne <roger.pau@citrix.com>; >>>> Stefano Stabellini <sstabellini@kernel.org>; xen-devel <xen- >>>> devel@lists.xenproject.org> >>>> Subject: RE: IOREQ server on Arm >>>> >>>>>>> On 26.09.18 at 13:02, <Paul.Durrant@citrix.com> wrote: >>>>> --- a/xen/common/memory.c >>>>> +++ b/xen/common/memory.c >>>>> @@ -1105,8 +1105,11 @@ static int acquire_resource( >>>>> >>>>> for ( i = 0; !rc && i < xmar.nr_frames; i++ ) >>>>> { >>>>> - rc = set_foreign_p2m_entry(currd, gfn_list[i], >>>>> - _mfn(mfn_list[i])); >>>>> + rc = (xmar.flags & XENMEM_rsrc_acq_caller_owned) ? >>>>> + guest_physmap_add_entry(currd, gfn_list[i], >>>>> + _mfn(mfn_list[i]), 0, >>>> p2m_ram_rw) : >>>>> + set_foreign_p2m_entry(currd, gfn_list[i], >>>>> + _mfn(mfn_list[i])); >>>>> /* rc should be -EIO for any iteration other than the >> first >>>> */ >>>>> if ( rc && i ) >>>>> rc = -EIO; >>>>> >>>>> But the guest_physmap_add_entry() is problematic as it will IOMMU map >>>> pages >>>>> as well, which is probably not wanted. >>>> >>>> Yeah, I'd prefer if we avoided establishing IOMMU mappings here. >>>> How about transforming set_foreign_p2m_entry() into >>>> set_special_p2m_entry(), with a type passed in? >>>> >>> >>> That sounds like it might work. >>> >>> Julien, do you want page types to distinguish caller-owned resources >> from normal RAM are you ok with p2m_ram_rw even though it could be subject >> of another domain's foreign map? >> >> Based on your previous e-mail, I would be fine with that on Arm. >> >> This brings me to the next question. Do you expect set_special_p2m_entry >> to take a reference on the page? >> >> If not, we may run into some troubles because AFAICT you can map twice >> the ioreq page in a guest but reference will only be taken on the >> allocation. >> >> However, the unmap path will always drop a reference when removing the >> page. This is because Xen at the moment, reference will not be taken on >> mapping but allocation (we assume a page could not be mapped twice in a >> guest). >> >> Foreign mapping on Arm are a bit special because we get a reference on >> mapping them and will drop it when the mapping disappear. So we would >> not have any problem there. >> >> Any thoughts? > > Well, as Jan says, on x86 we don't reference count in the P2M so multiple mappings should not be an issue AFAICT. I understand that you don't have reference count in the P2M (that's the same on Arm today except for foreign mapping). But I think I can list at least 2 major issues with the design today. Let me give an example based on my understanding. 1. DM requests to map the IOREQ page a) page allocated (one reference) b) get reference (will be dropped when the IOREQ server is destroyed) 2. DM requests to map the IOREQ page (second time) No reference taken 3. DM unmap the IOREQ page 4. DM unmap the IOREQ page AFAIU, 3. 4. would be done through XENMEM_remove_from_physmap. So no reference dropped there. While the reference 1.b) will be dropped in hvm_free_ioreq_mfn. AFAICT 1.a) would be kept until the domain die. This would result to Xen memory exhaustion in long term. Did I miss anything? But, I think there are another way for badly written guest to remove the page. It looks like you can use XENMEM_decrease_reservation as the page belongs to the guest. So a reference would be dropped by 3. and 4. While 3. will drop the reference drop by 1.a), 4. may drop the reference from 1.b) and releasing the page for good. Although the page will still be associated with the IOREQ server until it has been effectively destroyed. Did I miss anything in the code? > It sounds like resource mapping should be treated the same way as foreign mapping (albeit with a non-foreign domid) such that the reference acquisition occurs at map time. If my understanding is correct then yes it would be much safer to get reference here. Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: IOREQ server on Arm 2018-09-27 9:41 ` Julien Grall @ 2018-09-27 10:16 ` Paul Durrant 2018-09-27 10:31 ` Julien Grall 0 siblings, 1 reply; 23+ messages in thread From: Paul Durrant @ 2018-09-27 10:16 UTC (permalink / raw) To: 'Julien Grall', 'Jan Beulich' Cc: Andrew Cooper, Stefano Stabellini, xen-devel, Roger Pau Monne > -----Original Message----- > From: Julien Grall [mailto:julien.grall@arm.com] > Sent: 27 September 2018 10:42 > To: Paul Durrant <Paul.Durrant@citrix.com>; 'Jan Beulich' > <JBeulich@suse.com> > Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Roger Pau Monne > <roger.pau@citrix.com>; Stefano Stabellini <sstabellini@kernel.org>; xen- > devel <xen-devel@lists.xenproject.org> > Subject: Re: IOREQ server on Arm > > Hi Paul, > > On 09/27/2018 09:38 AM, Paul Durrant wrote: > >> -----Original Message----- > >> From: Julien Grall [mailto:julien.grall@arm.com] > >> Sent: 26 September 2018 22:32 > >> To: Paul Durrant <Paul.Durrant@citrix.com>; 'Jan Beulich' > >> <JBeulich@suse.com> > >> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Roger Pau Monne > >> <roger.pau@citrix.com>; Stefano Stabellini <sstabellini@kernel.org>; > xen- > >> devel <xen-devel@lists.xenproject.org> > >> Subject: Re: IOREQ server on Arm > >> > >> Hi Paul, > >> > >> On 09/26/2018 01:01 PM, Paul Durrant wrote: > >>>> -----Original Message----- > >>>> From: Jan Beulich [mailto:JBeulich@suse.com] > >>>> Sent: 26 September 2018 12:57 > >>>> To: Paul Durrant <Paul.Durrant@citrix.com> > >>>> Cc: Julien Grall <julien.grall@arm.com>; Andrew Cooper > >>>> <Andrew.Cooper3@citrix.com>; Roger Pau Monne <roger.pau@citrix.com>; > >>>> Stefano Stabellini <sstabellini@kernel.org>; xen-devel <xen- > >>>> devel@lists.xenproject.org> > >>>> Subject: RE: IOREQ server on Arm > >>>> > >>>>>>> On 26.09.18 at 13:02, <Paul.Durrant@citrix.com> wrote: > >>>>> --- a/xen/common/memory.c > >>>>> +++ b/xen/common/memory.c > >>>>> @@ -1105,8 +1105,11 @@ static int acquire_resource( > >>>>> > >>>>> for ( i = 0; !rc && i < xmar.nr_frames; i++ ) > >>>>> { > >>>>> - rc = set_foreign_p2m_entry(currd, gfn_list[i], > >>>>> - _mfn(mfn_list[i])); > >>>>> + rc = (xmar.flags & XENMEM_rsrc_acq_caller_owned) ? > >>>>> + guest_physmap_add_entry(currd, gfn_list[i], > >>>>> + _mfn(mfn_list[i]), 0, > >>>> p2m_ram_rw) : > >>>>> + set_foreign_p2m_entry(currd, gfn_list[i], > >>>>> + _mfn(mfn_list[i])); > >>>>> /* rc should be -EIO for any iteration other than the > >> first > >>>> */ > >>>>> if ( rc && i ) > >>>>> rc = -EIO; > >>>>> > >>>>> But the guest_physmap_add_entry() is problematic as it will IOMMU > map > >>>> pages > >>>>> as well, which is probably not wanted. > >>>> > >>>> Yeah, I'd prefer if we avoided establishing IOMMU mappings here. > >>>> How about transforming set_foreign_p2m_entry() into > >>>> set_special_p2m_entry(), with a type passed in? > >>>> > >>> > >>> That sounds like it might work. > >>> > >>> Julien, do you want page types to distinguish caller-owned resources > >> from normal RAM are you ok with p2m_ram_rw even though it could be > subject > >> of another domain's foreign map? > >> > >> Based on your previous e-mail, I would be fine with that on Arm. > >> > >> This brings me to the next question. Do you expect > set_special_p2m_entry > >> to take a reference on the page? > >> > >> If not, we may run into some troubles because AFAICT you can map twice > >> the ioreq page in a guest but reference will only be taken on the > >> allocation. > >> > >> However, the unmap path will always drop a reference when removing the > >> page. This is because Xen at the moment, reference will not be taken on > >> mapping but allocation (we assume a page could not be mapped twice in a > >> guest). > >> > >> Foreign mapping on Arm are a bit special because we get a reference on > >> mapping them and will drop it when the mapping disappear. So we would > >> not have any problem there. > >> > >> Any thoughts? > > > > Well, as Jan says, on x86 we don't reference count in the P2M so > multiple mappings should not be an issue AFAICT. > > I understand that you don't have reference count in the P2M (that's the > same on Arm today except for foreign mapping). But I think I can list at > least 2 major issues with the design today. Let me give an example based > on my understanding. > > 1. DM requests to map the IOREQ page > a) page allocated (one reference) > b) get reference (will be dropped when the IOREQ server is > destroyed) > > 2. DM requests to map the IOREQ page (second time) > No reference taken > > 3. DM unmap the IOREQ page > 4. DM unmap the IOREQ page > > AFAIU, 3. 4. would be done through XENMEM_remove_from_physmap. So no > reference dropped there. While the reference 1.b) will be dropped in > hvm_free_ioreq_mfn. AFAICT 1.a) would be kept until the domain die. This > would result to Xen memory exhaustion in long term. Did I miss anything? > 1.a) would be kept until the IOREQ server is destroyed, which will happen either at domain destruction *or* when the DM destroys it. > But, I think there are another way for badly written guest to remove the > page. It looks like you can use XENMEM_decrease_reservation as the page > belongs to the guest. So a reference would be dropped by 3. and 4. > How so? The pages don't belong to the guest; they belong the the DM domain. They never appear in guest P2M so how can the guest decrease_reservation them? Are you worried about the DM domain doing a decrease reservation? Paul > While 3. will drop the reference drop by 1.a), 4. may drop the reference > from 1.b) and releasing the page for good. Although the page will still > be associated with the IOREQ server until it has been effectively > destroyed. Did I miss anything in the code? > > > It sounds like resource mapping should be treated the same way as > foreign mapping (albeit with a non-foreign domid) such that the reference > acquisition occurs at map time. > > If my understanding is correct then yes it would be much safer to get > reference here. > > Cheers, > > -- > Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: IOREQ server on Arm 2018-09-27 10:16 ` Paul Durrant @ 2018-09-27 10:31 ` Julien Grall 2018-09-27 10:46 ` Paul Durrant 0 siblings, 1 reply; 23+ messages in thread From: Julien Grall @ 2018-09-27 10:31 UTC (permalink / raw) To: Paul Durrant, 'Jan Beulich' Cc: Andrew Cooper, Stefano Stabellini, xen-devel, Roger Pau Monne Hi Paul, On 09/27/2018 11:16 AM, Paul Durrant wrote: >> -----Original Message----- >> From: Julien Grall [mailto:julien.grall@arm.com] >> Sent: 27 September 2018 10:42 >> To: Paul Durrant <Paul.Durrant@citrix.com>; 'Jan Beulich' >> <JBeulich@suse.com> >> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Roger Pau Monne >> <roger.pau@citrix.com>; Stefano Stabellini <sstabellini@kernel.org>; xen- >> devel <xen-devel@lists.xenproject.org> >> Subject: Re: IOREQ server on Arm >> >> Hi Paul, >> >> On 09/27/2018 09:38 AM, Paul Durrant wrote: >>>> -----Original Message----- >>>> From: Julien Grall [mailto:julien.grall@arm.com] >>>> Sent: 26 September 2018 22:32 >>>> To: Paul Durrant <Paul.Durrant@citrix.com>; 'Jan Beulich' >>>> <JBeulich@suse.com> >>>> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Roger Pau Monne >>>> <roger.pau@citrix.com>; Stefano Stabellini <sstabellini@kernel.org>; >> xen- >>>> devel <xen-devel@lists.xenproject.org> >>>> Subject: Re: IOREQ server on Arm >>>> >>>> Hi Paul, >>>> >>>> On 09/26/2018 01:01 PM, Paul Durrant wrote: >>>>>> -----Original Message----- >>>>>> From: Jan Beulich [mailto:JBeulich@suse.com] >>>>>> Sent: 26 September 2018 12:57 >>>>>> To: Paul Durrant <Paul.Durrant@citrix.com> >>>>>> Cc: Julien Grall <julien.grall@arm.com>; Andrew Cooper >>>>>> <Andrew.Cooper3@citrix.com>; Roger Pau Monne <roger.pau@citrix.com>; >>>>>> Stefano Stabellini <sstabellini@kernel.org>; xen-devel <xen- >>>>>> devel@lists.xenproject.org> >>>>>> Subject: RE: IOREQ server on Arm >>>>>> >>>>>>>>> On 26.09.18 at 13:02, <Paul.Durrant@citrix.com> wrote: >>>>>>> --- a/xen/common/memory.c >>>>>>> +++ b/xen/common/memory.c >>>>>>> @@ -1105,8 +1105,11 @@ static int acquire_resource( >>>>>>> >>>>>>> for ( i = 0; !rc && i < xmar.nr_frames; i++ ) >>>>>>> { >>>>>>> - rc = set_foreign_p2m_entry(currd, gfn_list[i], >>>>>>> - _mfn(mfn_list[i])); >>>>>>> + rc = (xmar.flags & XENMEM_rsrc_acq_caller_owned) ? >>>>>>> + guest_physmap_add_entry(currd, gfn_list[i], >>>>>>> + _mfn(mfn_list[i]), 0, >>>>>> p2m_ram_rw) : >>>>>>> + set_foreign_p2m_entry(currd, gfn_list[i], >>>>>>> + _mfn(mfn_list[i])); >>>>>>> /* rc should be -EIO for any iteration other than the >>>> first >>>>>> */ >>>>>>> if ( rc && i ) >>>>>>> rc = -EIO; >>>>>>> >>>>>>> But the guest_physmap_add_entry() is problematic as it will IOMMU >> map >>>>>> pages >>>>>>> as well, which is probably not wanted. >>>>>> >>>>>> Yeah, I'd prefer if we avoided establishing IOMMU mappings here. >>>>>> How about transforming set_foreign_p2m_entry() into >>>>>> set_special_p2m_entry(), with a type passed in? >>>>>> >>>>> >>>>> That sounds like it might work. >>>>> >>>>> Julien, do you want page types to distinguish caller-owned resources >>>> from normal RAM are you ok with p2m_ram_rw even though it could be >> subject >>>> of another domain's foreign map? >>>> >>>> Based on your previous e-mail, I would be fine with that on Arm. >>>> >>>> This brings me to the next question. Do you expect >> set_special_p2m_entry >>>> to take a reference on the page? >>>> >>>> If not, we may run into some troubles because AFAICT you can map twice >>>> the ioreq page in a guest but reference will only be taken on the >>>> allocation. >>>> >>>> However, the unmap path will always drop a reference when removing the >>>> page. This is because Xen at the moment, reference will not be taken on >>>> mapping but allocation (we assume a page could not be mapped twice in a >>>> guest). >>>> >>>> Foreign mapping on Arm are a bit special because we get a reference on >>>> mapping them and will drop it when the mapping disappear. So we would >>>> not have any problem there. >>>> >>>> Any thoughts? >>> >>> Well, as Jan says, on x86 we don't reference count in the P2M so >> multiple mappings should not be an issue AFAICT. >> >> I understand that you don't have reference count in the P2M (that's the >> same on Arm today except for foreign mapping). But I think I can list at >> least 2 major issues with the design today. Let me give an example based >> on my understanding. >> >> 1. DM requests to map the IOREQ page >> a) page allocated (one reference) >> b) get reference (will be dropped when the IOREQ server is >> destroyed) >> >> 2. DM requests to map the IOREQ page (second time) >> No reference taken >> >> 3. DM unmap the IOREQ page >> 4. DM unmap the IOREQ page >> >> AFAIU, 3. 4. would be done through XENMEM_remove_from_physmap. So no >> reference dropped there. While the reference 1.b) will be dropped in >> hvm_free_ioreq_mfn. AFAICT 1.a) would be kept until the domain die. This >> would result to Xen memory exhaustion in long term. Did I miss anything? >> > > 1.a) would be kept until the IOREQ server is destroyed, which will happen either at domain destruction *or* when the DM destroys it. Argh, I mixed get_page_type and get_page(). Sorry for the noise. > >> But, I think there are another way for badly written guest to remove the >> page. It looks like you can use XENMEM_decrease_reservation as the page >> belongs to the guest. So a reference would be dropped by 3. and 4. >> > > How so? The pages don't belong to the guest; they belong the the DM domain. They never appear in guest P2M so how can the guest decrease_reservation them? Are you worried about the DM domain doing a decrease reservation? By guest I meant DM domain. Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: IOREQ server on Arm 2018-09-27 10:31 ` Julien Grall @ 2018-09-27 10:46 ` Paul Durrant 2018-09-27 10:55 ` Julien Grall 0 siblings, 1 reply; 23+ messages in thread From: Paul Durrant @ 2018-09-27 10:46 UTC (permalink / raw) To: 'Julien Grall', 'Jan Beulich' Cc: Andrew Cooper, Stefano Stabellini, xen-devel, Roger Pau Monne > -----Original Message----- > From: Julien Grall [mailto:julien.grall@arm.com] > Sent: 27 September 2018 11:31 > To: Paul Durrant <Paul.Durrant@citrix.com>; 'Jan Beulich' > <JBeulich@suse.com> > Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Roger Pau Monne > <roger.pau@citrix.com>; Stefano Stabellini <sstabellini@kernel.org>; xen- > devel <xen-devel@lists.xenproject.org> > Subject: Re: IOREQ server on Arm > > Hi Paul, > > On 09/27/2018 11:16 AM, Paul Durrant wrote: > >> -----Original Message----- > >> From: Julien Grall [mailto:julien.grall@arm.com] > >> Sent: 27 September 2018 10:42 > >> To: Paul Durrant <Paul.Durrant@citrix.com>; 'Jan Beulich' > >> <JBeulich@suse.com> > >> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Roger Pau Monne > >> <roger.pau@citrix.com>; Stefano Stabellini <sstabellini@kernel.org>; > xen- > >> devel <xen-devel@lists.xenproject.org> > >> Subject: Re: IOREQ server on Arm > >> > >> Hi Paul, > >> > >> On 09/27/2018 09:38 AM, Paul Durrant wrote: > >>>> -----Original Message----- > >>>> From: Julien Grall [mailto:julien.grall@arm.com] > >>>> Sent: 26 September 2018 22:32 > >>>> To: Paul Durrant <Paul.Durrant@citrix.com>; 'Jan Beulich' > >>>> <JBeulich@suse.com> > >>>> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Roger Pau Monne > >>>> <roger.pau@citrix.com>; Stefano Stabellini <sstabellini@kernel.org>; > >> xen- > >>>> devel <xen-devel@lists.xenproject.org> > >>>> Subject: Re: IOREQ server on Arm > >>>> > >>>> Hi Paul, > >>>> > >>>> On 09/26/2018 01:01 PM, Paul Durrant wrote: > >>>>>> -----Original Message----- > >>>>>> From: Jan Beulich [mailto:JBeulich@suse.com] > >>>>>> Sent: 26 September 2018 12:57 > >>>>>> To: Paul Durrant <Paul.Durrant@citrix.com> > >>>>>> Cc: Julien Grall <julien.grall@arm.com>; Andrew Cooper > >>>>>> <Andrew.Cooper3@citrix.com>; Roger Pau Monne > <roger.pau@citrix.com>; > >>>>>> Stefano Stabellini <sstabellini@kernel.org>; xen-devel <xen- > >>>>>> devel@lists.xenproject.org> > >>>>>> Subject: RE: IOREQ server on Arm > >>>>>> > >>>>>>>>> On 26.09.18 at 13:02, <Paul.Durrant@citrix.com> wrote: > >>>>>>> --- a/xen/common/memory.c > >>>>>>> +++ b/xen/common/memory.c > >>>>>>> @@ -1105,8 +1105,11 @@ static int acquire_resource( > >>>>>>> > >>>>>>> for ( i = 0; !rc && i < xmar.nr_frames; i++ ) > >>>>>>> { > >>>>>>> - rc = set_foreign_p2m_entry(currd, gfn_list[i], > >>>>>>> - _mfn(mfn_list[i])); > >>>>>>> + rc = (xmar.flags & XENMEM_rsrc_acq_caller_owned) ? > >>>>>>> + guest_physmap_add_entry(currd, gfn_list[i], > >>>>>>> + _mfn(mfn_list[i]), 0, > >>>>>> p2m_ram_rw) : > >>>>>>> + set_foreign_p2m_entry(currd, gfn_list[i], > >>>>>>> + _mfn(mfn_list[i])); > >>>>>>> /* rc should be -EIO for any iteration other than > the > >>>> first > >>>>>> */ > >>>>>>> if ( rc && i ) > >>>>>>> rc = -EIO; > >>>>>>> > >>>>>>> But the guest_physmap_add_entry() is problematic as it will IOMMU > >> map > >>>>>> pages > >>>>>>> as well, which is probably not wanted. > >>>>>> > >>>>>> Yeah, I'd prefer if we avoided establishing IOMMU mappings here. > >>>>>> How about transforming set_foreign_p2m_entry() into > >>>>>> set_special_p2m_entry(), with a type passed in? > >>>>>> > >>>>> > >>>>> That sounds like it might work. > >>>>> > >>>>> Julien, do you want page types to distinguish caller-owned resources > >>>> from normal RAM are you ok with p2m_ram_rw even though it could be > >> subject > >>>> of another domain's foreign map? > >>>> > >>>> Based on your previous e-mail, I would be fine with that on Arm. > >>>> > >>>> This brings me to the next question. Do you expect > >> set_special_p2m_entry > >>>> to take a reference on the page? > >>>> > >>>> If not, we may run into some troubles because AFAICT you can map > twice > >>>> the ioreq page in a guest but reference will only be taken on the > >>>> allocation. > >>>> > >>>> However, the unmap path will always drop a reference when removing > the > >>>> page. This is because Xen at the moment, reference will not be taken > on > >>>> mapping but allocation (we assume a page could not be mapped twice in > a > >>>> guest). > >>>> > >>>> Foreign mapping on Arm are a bit special because we get a reference > on > >>>> mapping them and will drop it when the mapping disappear. So we would > >>>> not have any problem there. > >>>> > >>>> Any thoughts? > >>> > >>> Well, as Jan says, on x86 we don't reference count in the P2M so > >> multiple mappings should not be an issue AFAICT. > >> > >> I understand that you don't have reference count in the P2M (that's the > >> same on Arm today except for foreign mapping). But I think I can list > at > >> least 2 major issues with the design today. Let me give an example > based > >> on my understanding. > >> > >> 1. DM requests to map the IOREQ page > >> a) page allocated (one reference) > >> b) get reference (will be dropped when the IOREQ server is > >> destroyed) > >> > >> 2. DM requests to map the IOREQ page (second time) > >> No reference taken > >> > >> 3. DM unmap the IOREQ page > >> 4. DM unmap the IOREQ page > >> > >> AFAIU, 3. 4. would be done through XENMEM_remove_from_physmap. So no > >> reference dropped there. While the reference 1.b) will be dropped in > >> hvm_free_ioreq_mfn. AFAICT 1.a) would be kept until the domain die. > This > >> would result to Xen memory exhaustion in long term. Did I miss > anything? > >> > > > > 1.a) would be kept until the IOREQ server is destroyed, which will > happen either at domain destruction *or* when the DM destroys it. > > Argh, I mixed get_page_type and get_page(). Sorry for the noise. > > > > >> But, I think there are another way for badly written guest to remove > the > >> page. It looks like you can use XENMEM_decrease_reservation as the page > >> belongs to the guest. So a reference would be dropped by 3. and 4. > >> > > > > How so? The pages don't belong to the guest; they belong the the DM > domain. They never appear in guest P2M so how can the guest > decrease_reservation them? Are you worried about the DM domain doing a > decrease reservation? > > By guest I meant DM domain. > If the DM domain is not PV then currently it must be the hardware domain to be able to map resources. Hence we trust it not to descrease_reservation IOREQ pages. Paul > Cheers, > > -- > Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: IOREQ server on Arm 2018-09-27 10:46 ` Paul Durrant @ 2018-09-27 10:55 ` Julien Grall 0 siblings, 0 replies; 23+ messages in thread From: Julien Grall @ 2018-09-27 10:55 UTC (permalink / raw) To: Paul Durrant, 'Jan Beulich' Cc: Andrew Cooper, Stefano Stabellini, xen-devel, Roger Pau Monne Hi Paul, Thank you for your help understanding the resource code. On 09/27/2018 11:46 AM, Paul Durrant wrote: > If the DM domain is not PV then currently it must be the hardware domain to be able to map resources. Hence we trust it not to descrease_reservation IOREQ pages. Can you point me to check in the code? The only check I found is: !(xmar.flags & XENMEM_rsrc_acq_caller_owned) && !is_hardware_domain(d) But this is still allowing to map some of the resources in a DM domain. Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: IOREQ server on Arm 2018-09-26 10:51 ` Paul Durrant 2018-09-26 10:58 ` Jan Beulich @ 2018-09-26 11:01 ` Julien Grall 2018-09-26 11:07 ` Paul Durrant 1 sibling, 1 reply; 23+ messages in thread From: Julien Grall @ 2018-09-26 11:01 UTC (permalink / raw) To: Paul Durrant, Jan Beulich Cc: Andrew Cooper, Stefano Stabellini, xen-devel, Roger Pau Monne Hi Paul, On 09/26/2018 11:51 AM, Paul Durrant wrote: >> -----Original Message----- >> From: Julien Grall [mailto:julien.grall@arm.com] >> Sent: 26 September 2018 11:41 >> To: Jan Beulich <JBeulich@suse.com>; Paul Durrant >> <Paul.Durrant@citrix.com> >> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Roger Pau Monne >> <roger.pau@citrix.com>; Stefano Stabellini <sstabellini@kernel.org>; xen- >> devel <xen-devel@lists.xenproject.org> >> Subject: Re: IOREQ server on Arm >> >> Hi Jan, >> >> On 09/26/2018 09:08 AM, Jan Beulich wrote:_ >>>>>> On 26.09.18 at 00:39, <julien.grall@arm.com> wrote: >>>> Hi Paul, >>>> >>>> I am looking at porting the IOREQ server infrastructure on Arm. I >> didn't >>>> need much modification to make it run for Arm. Although, the >>>> implementation could be simplified over the x86 implementation. >>>> >>>> I noticed some issue while trying to implement the hypercall >>>> XENMEM_acquire_resource. Per my understanding, all the page mapped via >>>> that hypercall will use the type p2m_mapping_foreign. >>>> >>>> This will result to trigger the ASSERT(fdom != dom) in >> get_page_from_gfn >>>> (asm-arm/p2m.h) because the IOREQ page has been allocated to the >>>> emulator domain and mapped to it. AFAICT x86 has the same assert in >>>> p2m_get_page_from_gfn(...). >>>> >>>> IHMO, the ASSERT makes sense because you are only meant to map page >>>> belonging to other domain with that type. >>>> >>>> So I am wondering whether IOREQ server running in PVH Dom0 has been >>>> tested? What would be the best course of action to fix the issue? >>> >>> I think the p2m type needs to be chosen based on >>> XENMEM_rsrc_acq_caller_owned. >> >> I am thinking to introduce p2m_mapping_owned. Or do we have a p2m_type >> that we could re-use? >> > > I think we should be able to just use p2m_ram_rw if it is caller owned. I thought about p2m_ram_rw but discarded because of the security implications. At least on Arm, this type can be used for foreign mapping, guest_copy helpers. This is not the case for p2m_mapping_foreign. Do we want to allow thoses resources to be used in hypercall buffer and/or mapped by other guest via the foreign API? If not, then we want to use a different type. Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: IOREQ server on Arm 2018-09-26 11:01 ` Julien Grall @ 2018-09-26 11:07 ` Paul Durrant 0 siblings, 0 replies; 23+ messages in thread From: Paul Durrant @ 2018-09-26 11:07 UTC (permalink / raw) To: 'Julien Grall', Jan Beulich Cc: Andrew Cooper, Stefano Stabellini, xen-devel, Roger Pau Monne > -----Original Message----- > From: Julien Grall [mailto:julien.grall@arm.com] > Sent: 26 September 2018 12:01 > To: Paul Durrant <Paul.Durrant@citrix.com>; Jan Beulich > <JBeulich@suse.com> > Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Roger Pau Monne > <roger.pau@citrix.com>; Stefano Stabellini <sstabellini@kernel.org>; xen- > devel <xen-devel@lists.xenproject.org> > Subject: Re: IOREQ server on Arm > > Hi Paul, > > On 09/26/2018 11:51 AM, Paul Durrant wrote: > >> -----Original Message----- > >> From: Julien Grall [mailto:julien.grall@arm.com] > >> Sent: 26 September 2018 11:41 > >> To: Jan Beulich <JBeulich@suse.com>; Paul Durrant > >> <Paul.Durrant@citrix.com> > >> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Roger Pau Monne > >> <roger.pau@citrix.com>; Stefano Stabellini <sstabellini@kernel.org>; > xen- > >> devel <xen-devel@lists.xenproject.org> > >> Subject: Re: IOREQ server on Arm > >> > >> Hi Jan, > >> > >> On 09/26/2018 09:08 AM, Jan Beulich wrote:_ > >>>>>> On 26.09.18 at 00:39, <julien.grall@arm.com> wrote: > >>>> Hi Paul, > >>>> > >>>> I am looking at porting the IOREQ server infrastructure on Arm. I > >> didn't > >>>> need much modification to make it run for Arm. Although, the > >>>> implementation could be simplified over the x86 implementation. > >>>> > >>>> I noticed some issue while trying to implement the hypercall > >>>> XENMEM_acquire_resource. Per my understanding, all the page mapped > via > >>>> that hypercall will use the type p2m_mapping_foreign. > >>>> > >>>> This will result to trigger the ASSERT(fdom != dom) in > >> get_page_from_gfn > >>>> (asm-arm/p2m.h) because the IOREQ page has been allocated to the > >>>> emulator domain and mapped to it. AFAICT x86 has the same assert in > >>>> p2m_get_page_from_gfn(...). > >>>> > >>>> IHMO, the ASSERT makes sense because you are only meant to map page > >>>> belonging to other domain with that type. > >>>> > >>>> So I am wondering whether IOREQ server running in PVH Dom0 has been > >>>> tested? What would be the best course of action to fix the issue? > >>> > >>> I think the p2m type needs to be chosen based on > >>> XENMEM_rsrc_acq_caller_owned. > >> > >> I am thinking to introduce p2m_mapping_owned. Or do we have a p2m_type > >> that we could re-use? > >> > > > > I think we should be able to just use p2m_ram_rw if it is caller owned. > > I thought about p2m_ram_rw but discarded because of the security > implications. At least on Arm, this type can be used for foreign > mapping, guest_copy helpers. This is not the case for p2m_mapping_foreign. > Not sure. The emulator has to have privilege over the target, so any domain having privilege over the emulator would have privilege over the target, wouldn't it? So I don't think there is any security issue. > Do we want to allow thoses resources to be used in hypercall buffer > and/or mapped by other guest via the foreign API? If not, then we want > to use a different type. > I can't think of a use-case where we would want that, but I'm not sure there is any particular problem allowing it. Same goes for the IOMMU mappings that I mentioned to Jan though... not really desirable but not necessarily a problem (for existing resource types). A new p2m type may well be a better option. Paul > Cheers, > > -- > Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2018-10-01 15:56 UTC | newest] Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-09-25 22:39 IOREQ server on Arm Julien Grall 2018-09-26 8:08 ` Jan Beulich 2018-09-26 9:14 ` Paul Durrant 2018-09-26 10:32 ` Julien Grall 2018-09-26 10:38 ` Andrew Cooper 2018-09-26 10:49 ` Paul Durrant 2018-10-01 15:56 ` Roger Pau Monné 2018-09-26 10:41 ` Julien Grall 2018-09-26 10:51 ` Paul Durrant 2018-09-26 10:58 ` Jan Beulich 2018-09-26 11:02 ` Paul Durrant 2018-09-26 11:57 ` Jan Beulich 2018-09-26 12:01 ` Paul Durrant 2018-09-26 21:32 ` Julien Grall 2018-09-27 6:10 ` Jan Beulich 2018-09-27 8:38 ` Paul Durrant 2018-09-27 9:41 ` Julien Grall 2018-09-27 10:16 ` Paul Durrant 2018-09-27 10:31 ` Julien Grall 2018-09-27 10:46 ` Paul Durrant 2018-09-27 10:55 ` Julien Grall 2018-09-26 11:01 ` Julien Grall 2018-09-26 11:07 ` Paul Durrant
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.