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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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.

Jan


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