Re: [PATCH v3 1/2] Differentiate IO/mem resources tracked by ioreq server

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

On 8/10/2015 6:57 PM, Paul Durrant wrote:
>> -----Original Message-----
>> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
>> Sent: 10 August 2015 11:56
>> To: Paul Durrant; Wei Liu; Yu Zhang
>> Cc: xen-devel@lists.xen.org; Ian Jackson; Stefano Stabellini; Ian Campbell;
>> Keir (Xen.org); jbeulich@suse.com; Kevin Tian; zhiyuan.lv@intel.com
>> Subject: Re: [PATCH v3 1/2] Differentiate IO/mem resources tracked by ioreq
>> server
>>
>> On 10/08/15 09:33, Paul Durrant wrote:
>>>> -----Original Message-----
>>>> From: Wei Liu [mailto:wei.liu2@citrix.com]
>>>> Sent: 10 August 2015 09:26
>>>> To: Yu Zhang
>>>> Cc: xen-devel@lists.xen.org; Paul Durrant; Ian Jackson; Stefano Stabellini;
>> Ian
>>>> Campbell; Wei Liu; Keir (Xen.org); jbeulich@suse.com; Andrew Cooper;
>>>> Kevin Tian; zhiyuan.lv@intel.com
>>>> Subject: Re: [PATCH v3 1/2] Differentiate IO/mem resources tracked by
>> ioreq
>>>> server
>>>>
>>>> On Mon, Aug 10, 2015 at 11:33:40AM +0800, Yu Zhang wrote:
>>>>> Currently in ioreq server, guest write-protected ram pages are
>>>>> tracked in the same rangeset with device mmio resources. Yet
>>>>> unlike device mmio, which can be in big chunks, the guest write-
>>>>> protected pages may be discrete ranges with 4K bytes each.
>>>>>
>>>>> This patch uses a seperate rangeset for the guest ram pages.
>>>>> And a new ioreq type, IOREQ_TYPE_MEM, is defined.
>>>>>
>>>>> Note: Previously, a new hypercall or subop was suggested to map
>>>>> write-protected pages into ioreq server. However, it turned out
>>>>> handler of this new hypercall would be almost the same with the
>>>>> existing pair - HVMOP_[un]map_io_range_to_ioreq_server, and there's
>>>>> already a type parameter in this hypercall. So no new hypercall
>>>>> defined, only a new type is introduced.
>>>>>
>>>>> Signed-off-by: Yu Zhang <yu.c.zhang@linux.intel.com>
>>>>> ---
>>>>>   tools/libxc/include/xenctrl.h    | 39 +++++++++++++++++++++++---
>>>>>   tools/libxc/xc_domain.c          | 59
>>>> ++++++++++++++++++++++++++++++++++++++--
>>>>
>>>> FWIW the hypercall wrappers look correct to me.
>>>>
>>>>> diff --git a/xen/include/public/hvm/hvm_op.h
>>>> b/xen/include/public/hvm/hvm_op.h
>>>>> index 014546a..9106cb9 100644
>>>>> --- a/xen/include/public/hvm/hvm_op.h
>>>>> +++ b/xen/include/public/hvm/hvm_op.h
>>>>> @@ -329,8 +329,9 @@ struct xen_hvm_io_range {
>>>>>       ioservid_t id;               /* IN - server id */
>>>>>       uint32_t type;               /* IN - type of range */
>>>>>   # define HVMOP_IO_RANGE_PORT   0 /* I/O port range */
>>>>> -# define HVMOP_IO_RANGE_MEMORY 1 /* MMIO range */
>>>>> +# define HVMOP_IO_RANGE_MMIO   1 /* MMIO range */
>>>>>   # define HVMOP_IO_RANGE_PCI    2 /* PCI segment/bus/dev/func
>> range
>>>> */
>>>>> +# define HVMOP_IO_RANGE_MEMORY 3 /* MEMORY range */
>>>> This looks problematic. Maybe you can get away with this because this is
>>>> a toolstack-only interface?
>>>>
>>> Indeed, the old name is a bit problematic. Presumably re-use like this
>> would require an interface version change and some if-defery.
>>
>> I assume it is an interface used by qemu, so this patch in its currently
>> state will break things.
>
> If QEMU were re-built against the updated header, yes.

Thank you, Andrew & Paul. :)
Are you referring to the xen_map/unmap_memory_section routines in QEMU?
I noticed they are called by xen_region_add/del in QEMU. And I wonder,
are these 2 routines used to track a memory region or to track a MMIO
region? If the region to be added is a MMIO, I guess the new interface
should be fine, but if it is memory region to be added into ioreq
server, maybe a patch in QEMU is necessary(e.g. use some if-defery for
this new interface version you suggested)?

Thanks
Yu
>
>    Paul
>
>>
>> ~Andrew
>