[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [V9 3/3] Differentiate IO/mem resources tracked by ioreq server





On 1/6/2016 5:58 PM, Jan Beulich wrote:
On 06.01.16 at 10:44, <Paul.Durrant@xxxxxxxxxx> wrote:
  -----Original Message-----
From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
Sent: 06 January 2016 08:59
To: Zhang Yu
Cc: Andrew Cooper; Paul Durrant; Wei Liu; Ian Jackson; Stefano Stabellini;
Kevin Tian; zhiyuan.lv@xxxxxxxxx; Shuai Ruan; xen-devel@xxxxxxxxxxxxx; Keir
(Xen.org)
Subject: Re: [Xen-devel] [V9 3/3] Differentiate IO/mem resources tracked by
ioreq server

On 31.12.15 at 10:33, <yu.c.zhang@xxxxxxxxxxxxxxx> wrote:
On 12/21/2015 10:45 PM, Jan Beulich wrote:
On 15.12.15 at 03:05, <shuai.ruan@xxxxxxxxxxxxxxx> wrote:
@@ -2593,6 +2597,16 @@ struct hvm_ioreq_server
*hvm_select_ioreq_server(struct domain *d,
           type = (p->type == IOREQ_TYPE_PIO) ?
                   HVMOP_IO_RANGE_PORT : HVMOP_IO_RANGE_MEMORY;
           addr = p->addr;
+        if ( type == HVMOP_IO_RANGE_MEMORY )
+        {
+             ram_page = get_page_from_gfn(d, p->addr >> PAGE_SHIFT,
+                                          &p2mt, P2M_UNSHARE);
+             if ( p2mt == p2m_mmio_write_dm )
+                 type = HVMOP_IO_RANGE_WP_MEM;
+
+             if ( ram_page )
+                 put_page(ram_page);
+        }

You evaluate the page's current type here - what if it subsequently
changes? I don't think it is appropriate to leave the hypervisor at
the mercy of the device model here.

Well. I do not quite understand your concern. :)
Here, the get_page_from_gfn() is used to determine if the addr is a MMIO
or a write-protected ram. If this p2m type is changed, it should be
triggered by the guest and device model, e.g. this RAM is not supposed
to be used as the graphic translation table. And it should be fine.
But I also wonder, if there's any other routine more appropriate to get
a p2m type from the gfn?

No, the question isn't the choice of method to retrieve the
current type, but the lack of measures against the retrieved
type becoming stale by the time you actually use it.

I don't think that issue is specific to this code. AFAIK nothing in the I/O
emulation system protects against a type change whilst a request is in
flight.
Also, what are the consequences of a change? Only that the wrong range type
is selected and the emulation goes to the wrong place. This may be a problem
for the VM but should cause no other problems.

Okay, I buy this argument, but I think it would help if that was spelled
out this way in the commit message.

Thank you, Paul & Jan. :)
A note will be added to explain this in the commit message in next
version.

Yu

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.