[Xen-devel] Ping: [PATCH RFC] x86/HVM: meet xentrace's expectations on emulation event data

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 09.08.18 at 11:09, <Paul.Durrant@xxxxxxxxxx> wrote:
>>  -----Original Message-----
>> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>> Sent: 09 August 2018 09:02
>> To: xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
>> Cc: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; Paul Durrant
>> <Paul.Durrant@xxxxxxxxxx>; George Dunlap <George.Dunlap@xxxxxxxxxx>
>> Subject: [PATCH RFC] x86/HVM: meet xentrace's expectations on emulation
>> event data
>> 
>> According to the logic in hvm_mmio_assist_process(), 64 bits of data are
>> expected with 64-bit addresses, and 32 bits of data with 32-bit ones. I
>> don't think this is very reasonable, but I'm also not going to touch the
>> consumer side, the more that it is anyway not very helpful for the code
>> here to only ever supply 32 bits of data (despite the field being 64
>> bits wide, and having been even in the 32-bit days of Xen).
> 
> I suspect the data field was 64 bits so it could hold addresses, and no-one 
> thought about 64 bits of immediate data (which of course you can only get 
> with MMIO) whereas you can have 64-bit addresses even with PIO. Anyway the 
> change LGTM...
> 
>> 
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> Reviewed-by: Paul Durrant <paul.durrant@xxxxxxxxxx>

Thanks.

George, strictly speaking I could put this in with just this, but
considering ...

>> ---
>> RFC: Untested; solely based on the observation of "(no data)" in a trace
>>      where it was entirely unclear why no data would have been available.
>>      I just so happened that the guest had more than 4Gb of memory, and
>>      hence addresses were not representable as 32-bit values.

... this I'd prefer to have your consent.

Jan

>> --- a/xen/arch/x86/hvm/emulate.c
>> +++ b/xen/arch/x86/hvm/emulate.c
>> @@ -30,7 +30,7 @@ struct hvmemul_cache
>>  static void hvmtrace_io_assist(const ioreq_t *p)
>>  {
>>      unsigned int size, event;
>> -    unsigned char buffer[12];
>> +    unsigned char buffer[16];
>> 
>>      if ( likely(!tb_init_done) )
>>          return;
>> @@ -47,8 +47,11 @@ static void hvmtrace_io_assist(const ior
>> 
>>      if ( !p->data_is_ptr )
>>      {
>> -        *(uint32_t *)&buffer[size] = p->data;
>> -        size += 4;
>> +        if ( size == 4 )
>> +            *(uint32_t *)&buffer[size] = p->data;
>> +        else
>> +            *(uint64_t *)&buffer[size] = p->data;
>> +        size *= 2;
>>      }
>> 
>>      trace_var(event, 0/*!cycles*/, size, buffer);
>> 
>> 
>> 





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