Re: [PATCH v2] x86/HVM: correct hvmemul_map_linear_addr() for multi-page case

["Jan Beulich" <JBeulich@suse.com>]

>>> On 20.09.18 at 16:13, <andrew.cooper3@citrix.com> wrote:
> On 20/09/18 14:39, Jan Beulich wrote:
>>>>> On 20.09.18 at 14:41, <andrew.cooper3@citrix.com> wrote:
>>> On 13/09/18 11:12, Jan Beulich wrote:
>>>> The function does two translations in one go for a single guest access.
>>>> Any failure of the first translation step (guest linear -> guest
>>>> physical), resulting in #PF, ought to take precedence over any failure
>>>> of the second step (guest physical -> host physical).
>>> Why?  What is the basis of this presumption?
>>>
>>> As far as what real hardware does...
>>>
>>> This test sets up a ballooned page and a read-only page.  I.e. a second
>>> stage fault on the first part of a misaligned access, and a first stage
>>> fault on the second part of the access.
>>>
>>> (d1) --- Xen Test Framework ---
>>> (d1) Environment: HVM 64bit (Long mode 4 levels)
>>> (d1) Test splitfault
>>> (d1) About to read
>>> (XEN) *** EPT qual 0000000000000181, gpa 000000000011cffc
>>> (d1) Reading PTR: got 00000000ffffffff
>>> (d1) About to write
>>> (XEN) *** EPT qual 0000000000000182, gpa 000000000011cffc
>>> (d1) ******************************
>>> (d1) PANIC: Unhandled exception at 0008:00000000001047e0
>>> (d1) Vec 14 #PF[-d-sWP] %cr2 000000000011d000
>>> (d1) ******************************
>>>
>>> The second stage fault is recognised first, which is contrary to your
>>> presumption, i.e. the code in its current form appears to be correct.
>> But the guest doesn't know about 2nd stage translation. In the
>> absence of it, the (1st stage / only) fault ought to occur before
>> any bus level actions would be taken.
> 
> You have not answered my question.
> 
> Why?  On what basis do you conclude that the behaviour you describe is
> "correct", especially now given evidence to the contrary?

As to the basis I'm taking: Without it spelled out anywhere, any
sensible behavior can be considered "correct". But let's look at the
steps unpatched code takes:

hvm_translate_get_page() for the tail of the first page produces
HVMTRANS_bad_gfn_to_mfn, so we bail from the loop, returning
NULL. The caller takes this as an indication to write the range in
pieces. Hence a write to the last bytes of the first page occurs (if
it was MMIO instead of a ballooned page) before we raise #PF.

Now let's look at patched code behavior:

hvm_translate_get_page() for the tail of the first page produces
HVMTRANS_bad_gfn_to_mfn again, but we continue the loop.
hvm_translate_get_page() for the start of the second page
produces HVMTRANS_bad_linear_to_gfn, so we raise #PF without
first doing a partial write.

I continue to think that this is the less surprising behavior. Without
it being mandated that the partial write _has_ to occur, I'd much
prefer this changed behavior, no matter how the specific piece of
hardware behaves that you ran your test on.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel