On 13/11/2018 10:53, Paul Durrant wrote:
>> -----Original Message-----
>> From: Andrew Cooper
>> Sent: 13 November 2018 10:47
>> To: Jan Beulich <jbeul...@suse.com>; xen-devel <xen-
>> de...@lists.xenproject.org>
>> Cc: Paul Durrant <paul.durr...@citrix.com>; Wei Liu <wei.l...@citrix.com>
>> Subject: Re: [PATCH 1/3] x86/HVM: __hvm_copy() should not write to
>> p2m_ioreq_server pages
>>
>> On 13/11/18 10:13, Jan Beulich wrote:
>>> Commit 3bdec530a5 ("x86/HVM: split page straddling emulated accesses in
>>> more cases") introduced a hvm_copy_to_guest_linear() attempt before
>>> falling back to hvmemul_linear_mmio_write(). This is wrong for the
>>> p2m_ioreq_server special case. That change widened a pre-existing issue
>>> though: Other writes to such pages also need to be failed (or forced
>>> through emulation), in particular hypercall buffer writes.
>>>
>>> Reported-by: ??? <???@citrix.com>
>>> Signed-off-by: Jan Beulich <jbeul...@suse.com>
>>>
>>> --- a/xen/arch/x86/hvm/hvm.c
>>> +++ b/xen/arch/x86/hvm/hvm.c
>>> @@ -3202,6 +3202,12 @@ static enum hvm_translation_result __hvm
>>> if ( res != HVMTRANS_okay )
>>> return res;
>>>
>>> + if ( (flags & HVMCOPY_to_guest) && p2mt == p2m_ioreq_server )
>> While this does address the issue, I'm concerned about hardcoding the
>> behaviour here.
>>
>> p2m_ioreq_server doesn't mean "I want shadowing properties". It has an
>> as-yet unspecified per-ioreq-client meaning.
>>
>> We either want to rename p2m_ioreq_server to something which indicates
>> its "allow-reads/emulate writes" behaviour, or design a way for the
>> ioreq client to specify the behaviour it wants.
>>
> The comment in the public header is:
>
> /*
> * XEN_DMOP_map_mem_type_to_ioreq_server : map or unmap the IOREQ Server <id>
>
> * to specific memory type <type>
> * for specific accesses <flags>
> *
> * For now, flags only accept the value of XEN_DMOP_IOREQ_MEM_ACCESS_WRITE,
> * which means only write operations are to be forwarded to an ioreq server.
> * Support for the emulation of read operations can be added when an ioreq
> * server has such requirement in future.
> */
>
> ...so the write-intercept-only behaviour is baked in. Whilst I agree it would
> be nice not to proliferate this, I don't think it needs addressing in the
> short term.
Lovely :(
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
