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

["Yu, Zhang" <yu.c.zhang@linux.intel.com>]

On 1/6/2016 5:58 PM, Jan Beulich wrote:
>>>> On 06.01.16 at 10:44, <Paul.Durrant@citrix.com> wrote:
>>>   -----Original Message-----
>>> From: Jan Beulich [mailto:JBeulich@suse.com]
>>> Sent: 06 January 2016 08:59
>>> To: Zhang Yu
>>> Cc: Andrew Cooper; Paul Durrant; Wei Liu; Ian Jackson; Stefano Stabellini;
>>> Kevin Tian; zhiyuan.lv@intel.com; Shuai Ruan; xen-devel@lists.xen.org; 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@linux.intel.com> wrote:
>>>> On 12/21/2015 10:45 PM, Jan Beulich wrote:
>>>>>>>> On 15.12.15 at 03:05, <shuai.ruan@linux.intel.com> 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@lists.xen.org
> http://lists.xen.org/xen-devel
>