Hi Paul,
On 09/27/2018 11:16 AM, Paul Durrant wrote:
-----Original Message-----
From: Julien Grall [mailto:julien.gr...@arm.com]
Sent: 27 September 2018 10:42
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/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?
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
