Re: [PATCH v4 01/17] x86emul: support most memory accessing MMX/SSE{, 2, 3} insns

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 01/03/17 13:50, Jan Beulich wrote:
>
>> This seems rather inconsistent at the moment.
> Does it? At least in this patch I can't spot an inconsistency.

I meant in general across the current use of stubs, but perhaps that is
just because of the transition to the new model.

>
>>> @@ -6159,6 +6551,76 @@ x86_emulate(
>>>          goto cannot_emulate;
>>>      }
>>>  
>>> +    if ( state->simd_size )
>>> +    {
>>> +#ifdef __XEN__
>>> +        uint8_t *buf = stub.ptr;
>>> +#else
>>> +        uint8_t *buf = get_stub(stub);
>>> +#endif
>> Is this stale?  Everywhere else is just get_stub() without any ifdefary.
> No, it's not stale: In the hypervisor we can't use get_stub() a
> second time, or else we'll invoke map_domain_page() a second
> time, discarding (and hence leaking) the result of the earlier
> one.

As an aside, shouldn't we refcount (and at least assert) that we don't
map the same page twice, to avoid leaks?

>  And in the harness using get_stub() is the cleanest way
> to get hold of the pointer again. I've considered and tried
> several variants, but I couldn't come up with an approach not
> needing any #ifdef - if you see a way, let me know.

As it drops out in the following patch, I am not overly fussed.

~Andrew

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