Re: [PATCH v2 3/3] x86/ioreq server: Add HVMOP to map guest ram with p2m_ioreq_server to an ioreq server

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

On 4/12/2016 11:08 PM, Jan Beulich wrote:
>>>> "Yu, Zhang" <yu.c.zhang@linux.intel.com> 04/12/16 11:47 AM >>>
>> On 4/12/2016 12:31 AM, Jan Beulich wrote:
>>>>>> On 11.04.16 at 13:14, <yu.c.zhang@linux.intel.com> wrote:
>>>> On 4/9/2016 6:28 AM, Jan Beulich wrote:
>>>>>>>> On 31.03.16 at 12:53, <yu.c.zhang@linux.intel.com> wrote:
>>>>>> +        if ( s->id == id )
>>>>>> +        {
>>>>>> +            rc = p2m_set_ioreq_server(d, flags, s);
>>>>>> +            if ( rc == 0 )
>>>>>> +                gdprintk(XENLOG_DEBUG, "%u %s type HVMMEM_ioreq_server.\n",
>>>>>> +                         s->id, (flags != 0) ? "mapped to" : "unmapped from");
>>>>>
>>>>> Why gdprintk()? I don't think the current domain is of much
>>>>> interest here. What would be of interest is the subject domain.
>>>>>
>>>>
>>>> s->id is not the domain_id, but id of the ioreq server.
>>>
>>> That's understood. But gdprintk() itself logs the current domain,
>>> which isn't as useful as the subject one.
>>
>> Oh, I see. So the correct routine here should be dprintk(), right?
>
> Yes.
>
>>>>> And with all three bits now possibly being clear, aren't we risking the
>>>>> entries to be mis-treated as not-present ones?
>>>>
>>>> Hah. You got me. Thanks! :)
>>>> Now I realized it would be difficult if we wanna to emulate the read
>>>> operations for HVM. According to Intel mannual, entry->r is to be
>>>> cleared, so should entry->w if we do not want ept misconfig. And
>>>> with both read and write permissions being forbidden, entry->x can be
>>>> set only on processors with EXECUTE_ONLY capability.
>>>> To avoid any entry to be mis-treated as not-present. We have several
>>>> solutions:
>>>> a> do not support the read emulation for now - we have no such usage
>>>> case;
>>>> b> add the check of p2m_t against p2m_ioreq_server in is_epte_present -
>>>> a bit weird to me.
>>>> Which one do you prefer? or any other suggestions?
>>>
>>> That question would also need to be asked to others who had
>>> suggested supporting both. I'd be fine with a, but I also don't view
>>> b as too awkward.
>>
>> According to Intel mannual, an entry is regarded as not present, if
>> bit0:2 is 0. So adding a p2m type check in is_epte_present() means we
>> will change its semantics, if this is acceptable(with no hurt to
>> hypervisor). I'd prefer option b>
>
> Perhaps time for the VMX maintainers to chime in - such a change is acceptable
> only if it doesn't result in changed hardware behavior. I can't think of any such off
> the top of my head, but this really should be confirmed by the maintainers before
> deciding to go such a route.
>

Thanks, Jan. :)
Jun & Kevin, any suggestions?

Yu

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