Re: [Xen-devel] [PATCH 1/2] x86/HVM: drop hvm_fetch_from_guest_linear()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 30.08.18 at 14:22, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 30/08/18 13:02, Jan Beulich wrote:
>>>>> On 30.08.18 at 13:18, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> On 30/08/18 12:09, Jan Beulich wrote:
>>>> @@ -2512,9 +2512,10 @@ void hvm_emulate_init_per_insn(
>>>>                                          hvm_access_insn_fetch,
>>>>                                          
>>>> &hvmemul_ctxt->seg_reg[x86_seg_cs],
>>>>                                          &addr) &&
>>>> -             hvm_fetch_from_guest_linear(hvmemul_ctxt->insn_buf, addr,
>>>> -                                         sizeof(hvmemul_ctxt->insn_buf),
>>>> -                                         pfec, NULL) == HVMTRANS_okay) ?
>>>> +             hvm_copy_from_guest_linear(hvmemul_ctxt->insn_buf, addr,
>>>> +                                        sizeof(hvmemul_ctxt->insn_buf),
>>>> +                                        pfec | PFEC_insn_fetch, NULL,
>>>> +                                        NULL) == HVMTRANS_okay) ?
>>> Does this even compile?  You seem to have an extra NULL here and several
>>> later places.
>> It does - with "x86/HVM: implement memory read caching" also
>> applied. IOW - I'm sorry, insufficient re-ordering work done
>> when moving these two ahead.
> 
> Does it?  This patch has a mix of callers with 4 and 5 parameters, which
> is why I noticed it in the first place.

Right, hence the need for that other (re-based) patch on top. I believe
I've now moved things suitably between both patches.

> With it fixed up to compile, and preferably with the other adjustment
> included, Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Thanks (and yes, I've followed the other suggestion too),
Jan



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