Re: [Xen-devel] [PATCH v3 05/18] x86/hvm: remove multiple open coded 'chunking' loops

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 24.06.15 at 12:10, <Paul.Durrant@xxxxxxxxxx> wrote:
>> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>> Sent: 24 June 2015 10:38
>> >>> On 23.06.15 at 12:39, <paul.durrant@xxxxxxxxxx> wrote:
>> > --- a/xen/arch/x86/hvm/emulate.c
>> > +++ b/xen/arch/x86/hvm/emulate.c
>> > @@ -540,6 +540,115 @@ static int hvmemul_virtual_to_linear(
>> >      return X86EMUL_EXCEPTION;
>> >  }
>> >
>> > +static int hvmemul_phys_mmio_access(
>> > +    paddr_t gpa, unsigned int size, uint8_t dir, uint8_t *buffer,
>> > +    unsigned int *off)
>> 
>> Why this (buffer, off) pair? The caller can easily adjust buffer as
>> necessary, avoiding the other parameter altogether. And buffer
>> itself can be void * just like it is in some of the callers (and the
>> others should follow suit).
>> 
> 
> It actually becomes necessary in a later patch, but I'll make the change 
> there instead. As for incrementing a void *, I know that MSVC disallows this. 
> I believe it is a gcc-ism which I guess clang must tolerate, but I don't 
> think 
> it is standard C.

That's correct.

>> Also, considering instruction characteristics (as explained in the
>> original comment) I think the old way of determining the chunk
>> size may have been cheaper than yours using fls().
>> 
> 
> I thought fls() was generally implemented using inline assembler and was 
> pretty fast. I didn't actually check how Xen implements it; I just assumed it 
> would be optimal.

It uses BSR, which I remember to generally be (perhaps much) slower
than "ordinary" logical operations. But maybe that's not the case
anymore on recent CPUs...

Jan


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