Hi Paul,
On 09/27/2018 09:38 AM, Paul Durrant wrote:
-----Original Message-----
From: Julien Grall [mailto:julien.gr...@arm.com]
Sent: 26 September 2018 22:32
To: Paul Durrant <paul.durr...@citrix.com>; 'Jan Beulich'
<jbeul...@suse.com>
Cc: Andrew Cooper <andrew.coop...@citrix.com>; Roger Pau Monne
<roger....@citrix.com>; Stefano Stabellini <sstabell...@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:jbeul...@suse.com]
Sent: 26 September 2018 12:57
To: Paul Durrant <paul.durr...@citrix.com>
Cc: Julien Grall <julien.gr...@arm.com>; Andrew Cooper
<andrew.coop...@citrix.com>; Roger Pau Monne <roger....@citrix.com>;
Stefano Stabellini <sstabell...@kernel.org>; xen-devel <xen-
de...@lists.xenproject.org>
Subject: RE: IOREQ server on Arm
On 26.09.18 at 13:02, <paul.durr...@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
