Re: [PATCH v11 2/3] Differentiate IO/mem resources tracked by ioreq server

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

On 1/26/2016 7:24 PM, Jan Beulich wrote:
>>>> On 26.01.16 at 08:59, <yu.c.zhang@linux.intel.com> wrote:
>
>>
>> On 1/22/2016 7:43 PM, Jan Beulich wrote:
>>>>>> On 22.01.16 at 04:20, <yu.c.zhang@linux.intel.com> wrote:
>>>> @@ -2601,6 +2605,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);
>>>
>>> It seems to me like I had asked before: Why P2M_UNSHARE instead
>>> of just P2M_QUERY? (This could surely be fixed up while committing,
>>> the more that I've already done some cleanup here, but I'd like to
>>> understand this before it goes in.)
>>>
>> Hah, sorry for my bad memory. :)
>> I did not found P2M_QUERY; only P2M_UNSHARE and P2M_ALLOC are
>> defined. But after reading the code in ept_get_entry(), I guess the
>> P2M_UNSHARE is not accurate, maybe I should use 0 here for the
>> p2m_query_t parameter in get_page_from_gfn()?
>
> Ah, sorry for the misnamed suggestion. I'm not sure whether using
> zero here actually matches your needs; P2M_UNSHARE though
> seems odd in any case, so at least switching to P2M_ALLOC (to
> populate PoD pages) would seem to be necessary.
>

Thanks, Jan.  :)
And now I believe we should use zero here. By now XenGT does not
support PoD and here all we care about is whether the p2m type of this
gfn is p2m_mmio_write_dm.

>>>> @@ -2642,6 +2656,11 @@ struct hvm_ioreq_server *hvm_select_ioreq_server(struct domain *d,
>>>>                }
>>>>
>>>>                break;
>>>> +        case HVMOP_IO_RANGE_WP_MEM:
>>>> +            if ( rangeset_contains_singleton(r, PFN_DOWN(addr)) )
>>>> +                return s;
>>>
>>> Considering you've got p2m_mmio_write_dm above - can this
>>> validly return false here?
>>
>> Well, if we have multiple ioreq servers defined, it will...
>
> Ah, right. That's fine then.
>
> Jan
>
>

B.R.
Yu