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

[Igor Druzhinin <igor.druzhinin@xxxxxxxxxx>]

On 08/03/2019 14:58, Jan Beulich wrote:
>>>> On 08.03.19 at 15:25, <igor.druzhinin@xxxxxxxxxx> wrote:
>> On 08/03/2019 11:55, Jan Beulich wrote:
>>
>> 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.

What if we don't do hvm_copy() in that case and will go to slow-path
directly, would that be better?

> 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.

I don't think it's a good idea to transfer IOREQ state in different
places - IOREQ consumption happens now in hvmemul_do_io() function and
we need to re-enter it to finally finish with IOREQ processing. If we
want to change it - the entire infrastructure needs to be re-architected.

> 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.

Seems reasonable, I'd also prefer the solution to be symmetric.

> 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.)
I think the idea of completing IOREQ if it's still in progress is
general enough (we could also look for other possibly latent places in
the code where we bail prematurely).

Igor

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