Re: [Xen-devel] XenGT is still regressed on master

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 08.03.19 at 15:25, <igor.druzhinin@xxxxxxxxxx> wrote:
> On 08/03/2019 11:55, Jan Beulich wrote:
>> Assuming the p2m type change arrives via XEN_DMOP_set_mem_type,
>> I think what we need to do is delay the actual change until no ioreq
>> is pending anymore, kind of like the VM event subsystem delays
>> certain CR and MSR writes until VM entry time. In this situation we'd
>> then further have to restrict the number of such pending changes,
>> because we need to record the request(s) somewhere. Question is
>> how many of such bufferable requests would be enough: Unless we
>> wanted to enforce that only ordinary aligned writes (and rmw-s) can
>> get us into this state, apart from writes crossing page boundaries we
>> may need to be able to cope with AVX512's scatter insns.
>> 
>> The other alternative I can think of would be to record the specific
>> failure case (finding p2m_ioreq_server) in the first pass, and act upon
>> it (instead of your mitigation above) during the retry. At the first
>> glance it would seem more cumbersome to do the recording at that
>> layer though.
> 
> I like the latter suggestion more. Seems less invasive and prone to
> regressions. I'd like to try to implement it. Although I think the
> hypervisor check should be more general: like if IOREQ is in progress
> don't try to got through fast-path and re-enter IOREQ completion path.
> 
> What if we just check !hvm_ioreq_needs_completion() before returning
> X86EMUL_OKAY i.e. fall through to the bad_gfn_to_mfn case if that check
> fails as Paul suggested?

I didn't see such a suggestion, I think, and I'm afraid it would still not
be correct in the general case. As said before, Getting back
HVMTRANS_okay means the write did actually complete, and no
second attempt to do the write should be done.

If anything I could see the code transition from STATE_IORESP_READY
to STATE_IOREQ_NONE in that case, with a comment throughly
explaining why that's needed.

Independent of the specific solution - any change to linear_write()
would imo want suitably mirroring to linear_read(), which implies that
anything that can't validly be done to the latter is also unlikely to be
valid for the former.

And then, following Andrew's response, wouldn't what you suggest
again take care of only the p2m_ioreq_server special case? (Since
the immediate issue is with just this type, such a special case fix
might still be acceptable, as long as it comes with a promise to deal
with the general case down the road.)

Jan



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