Re: [Xen-devel] [PATCH RFC v3 2/2] x86/emulate: Send vm_event from emulate

[Alexandru Stefan ISAILA <aisaila@xxxxxxxxxxxxxxx>]

On 08.04.2019 18:32, Jan Beulich wrote:
>>>> On 06.02.19 at 13:53, <aisaila@xxxxxxxxxxxxxxx> wrote:
>> This patch aims to have mem access vm events sent from the emulator.
>> This is useful in the case of page-walks that have to emulate
>> instructions in access denied pages.
> 
> I'm afraid that I can't make sense of this: How could "page-walks
> have to emulate instructions"? Instructions can (and actually will)
> cause page walks to occur. And page walks hitting access denied
> pages may trigger emulation of the insn having initiated the walk.

I aimed for the idea that an emulated instruction could cause a 
page-walk that in the end hits protected pages. I will correct that part 
of the comment.

> 
>> We use hvmemul_map_linear_addr() ro intercept r/w access and
>> hvmemul_insn_fetch() to intercept exec access.
>>
>> First we try to send a vm event and if the event is sent then emulation
>> returns X86EMUL_ACCESS_EXCEPTION. If the event is not sent then the
>> emulation goes on as expected.
> 
> The meaning of this new emulator return value needs explanation.
> I notice its #define is also not accompanied by any comment. And
> any addition of a new emulator return code should come with a
> discussion of how existing users are affected. I'm not going to
> exclude that indeed no other adjustments are necessary, but that's
> far from obvious. You may recall that it had taken several iterations
> to get the addition of X86EMUL_UNIMPLEMENTED right throughout
> the code base.

This new feature is activated by "bool send_event" when calling 
hvm_emulate_one_vm_event(). Events will be sent in specific moments and 
only if it's an intention for that.

> 
> Overall I guess I'm simply not deeply enough into vm-event to
> be able to judge whether / how all of this makes sense.
> 
>> @@ -530,6 +532,55 @@ static int hvmemul_do_mmio_addr(paddr_t mmio_gpa,
>>       return hvmemul_do_io_addr(1, mmio_gpa, reps, size, dir, df, ram_gpa);
>>   }
>>   
>> +static bool hvmemul_send_vm_event(paddr_t gpa, unsigned long gla, gfn_t gfn,
>> +                                  uint32_t pfec, struct hvm_emulate_ctxt 
>> *ctxt)
> 
> Why both gpa and gfn?

If the gpa can be calculated from gfn then the code will be simplified.
Is this what you had in mind?

gpa = gfn_to_gaddr(gfn);

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