Re: [Xen-devel] [PATCH v3 23/25] x86/HVM: make use of new read-modify-write emulator hook

[Andrew Cooper]

On 05/02/18 08:34, Jan Beulich wrote:
 On 02.02.18 at 17:37,  wrote:
>> On 07/12/17 14:17, Jan Beulich wrote:
>>> ..., at least as far as currently possible, i.e. when a mapping can be
>>> obtained.
>>>
>>> Signed-off-by: Jan Beulich 
>>> ---
>>> v3: New.
>>>
>>> --- a/xen/arch/x86/hvm/emulate.c
>>> +++ b/xen/arch/x86/hvm/emulate.c
>>> @@ -1187,6 +1187,61 @@ static int hvmemul_write(
>>>  return X86EMUL_OKAY;
>>>  }
>>>  
>>> +static int hvmemul_rmw(
>>> +enum x86_segment seg,
>>> +unsigned long offset,
>>> +unsigned int bytes,
>>> +uint32_t *eflags,
>>> +struct x86_emulate_state *state,
>>> +struct x86_emulate_ctxt *ctxt)
>>> +{
>>> +struct hvm_emulate_ctxt *hvmemul_ctxt =
>>> +container_of(ctxt, struct hvm_emulate_ctxt, ctxt);
>>> +unsigned long addr, reps = 1;
>>> +uint32_t pfec = PFEC_page_present | PFEC_write_access;
>> Drop present, and...
> See reply to previous patch.
>
>>> +struct hvm_vcpu_io *vio = >arch.hvm_vcpu.hvm_io;
>>> +int rc;
>>> +void *mapping;
>>> +
>>> +rc = hvmemul_virtual_to_linear(
>>> +seg, offset, bytes, , hvm_access_write, hvmemul_ctxt, );
>>> +if ( rc != X86EMUL_OKAY || !bytes )
>>> +return rc;
>>> +
>>> +if ( is_x86_system_segment(seg) )
>>> +pfec |= PFEC_implicit;
>>> +else if ( hvmemul_ctxt->seg_reg[x86_seg_ss].dpl == 3 )
>>> +pfec |= PFEC_user_mode;
>>> +
>>> +mapping = hvmemul_map_linear_addr(addr, bytes, pfec, hvmemul_ctxt);
>>> +if ( IS_ERR(mapping) )
>>> +return ~PTR_ERR(mapping);
>>> +
>>> +if ( mapping )
>>> +{
>>> +rc = x86_emul_rmw(mapping, bytes, eflags, state, ctxt);
>>> +hvmemul_unmap_linear_addr(mapping, addr, bytes, hvmemul_ctxt);
>>> +}
>>> +else
>>> +{
>>> +unsigned long data = 0;
>>> +bool_t known_gpfn = vio->mmio_access.write_access &&
>>> +vio->mmio_gla == (addr & PAGE_MASK);
>> ... bool here.
> Oops.
>
>> Otherwise, Reviewed-by: Andrew Cooper 
> Thanks, but due to the first of the two requests you'll need to let
> me know whether this applies with just the second change done.

I'll accept the consistency argument and let the first one go.

~Andrew

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

Re: [Xen-devel] [PATCH v3 23/25] x86/HVM: make use of new read-modify-write emulator hook

[Jan Beulich]

>>> On 02.02.18 at 17:37,  wrote:
> On 07/12/17 14:17, Jan Beulich wrote:
>> ..., at least as far as currently possible, i.e. when a mapping can be
>> obtained.
>>
>> Signed-off-by: Jan Beulich 
>> ---
>> v3: New.
>>
>> --- a/xen/arch/x86/hvm/emulate.c
>> +++ b/xen/arch/x86/hvm/emulate.c
>> @@ -1187,6 +1187,61 @@ static int hvmemul_write(
>>  return X86EMUL_OKAY;
>>  }
>>  
>> +static int hvmemul_rmw(
>> +enum x86_segment seg,
>> +unsigned long offset,
>> +unsigned int bytes,
>> +uint32_t *eflags,
>> +struct x86_emulate_state *state,
>> +struct x86_emulate_ctxt *ctxt)
>> +{
>> +struct hvm_emulate_ctxt *hvmemul_ctxt =
>> +container_of(ctxt, struct hvm_emulate_ctxt, ctxt);
>> +unsigned long addr, reps = 1;
>> +uint32_t pfec = PFEC_page_present | PFEC_write_access;
> 
> Drop present, and...

See reply to previous patch.

>> +struct hvm_vcpu_io *vio = >arch.hvm_vcpu.hvm_io;
>> +int rc;
>> +void *mapping;
>> +
>> +rc = hvmemul_virtual_to_linear(
>> +seg, offset, bytes, , hvm_access_write, hvmemul_ctxt, );
>> +if ( rc != X86EMUL_OKAY || !bytes )
>> +return rc;
>> +
>> +if ( is_x86_system_segment(seg) )
>> +pfec |= PFEC_implicit;
>> +else if ( hvmemul_ctxt->seg_reg[x86_seg_ss].dpl == 3 )
>> +pfec |= PFEC_user_mode;
>> +
>> +mapping = hvmemul_map_linear_addr(addr, bytes, pfec, hvmemul_ctxt);
>> +if ( IS_ERR(mapping) )
>> +return ~PTR_ERR(mapping);
>> +
>> +if ( mapping )
>> +{
>> +rc = x86_emul_rmw(mapping, bytes, eflags, state, ctxt);
>> +hvmemul_unmap_linear_addr(mapping, addr, bytes, hvmemul_ctxt);
>> +}
>> +else
>> +{
>> +unsigned long data = 0;
>> +bool_t known_gpfn = vio->mmio_access.write_access &&
>> +vio->mmio_gla == (addr & PAGE_MASK);
> 
> ... bool here.

Oops.

> Otherwise, Reviewed-by: Andrew Cooper 

Thanks, but due to the first of the two requests you'll need to let
me know whether this applies with just the second change done.

Jan


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

Re: [Xen-devel] [PATCH v3 23/25] x86/HVM: make use of new read-modify-write emulator hook

[Andrew Cooper]

On 07/12/17 14:17, Jan Beulich wrote:
> ..., at least as far as currently possible, i.e. when a mapping can be
> obtained.
>
> Signed-off-by: Jan Beulich 
> ---
> v3: New.
>
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -1187,6 +1187,61 @@ static int hvmemul_write(
>  return X86EMUL_OKAY;
>  }
>  
> +static int hvmemul_rmw(
> +enum x86_segment seg,
> +unsigned long offset,
> +unsigned int bytes,
> +uint32_t *eflags,
> +struct x86_emulate_state *state,
> +struct x86_emulate_ctxt *ctxt)
> +{
> +struct hvm_emulate_ctxt *hvmemul_ctxt =
> +container_of(ctxt, struct hvm_emulate_ctxt, ctxt);
> +unsigned long addr, reps = 1;
> +uint32_t pfec = PFEC_page_present | PFEC_write_access;

Drop present, and...

> +struct hvm_vcpu_io *vio = >arch.hvm_vcpu.hvm_io;
> +int rc;
> +void *mapping;
> +
> +rc = hvmemul_virtual_to_linear(
> +seg, offset, bytes, , hvm_access_write, hvmemul_ctxt, );
> +if ( rc != X86EMUL_OKAY || !bytes )
> +return rc;
> +
> +if ( is_x86_system_segment(seg) )
> +pfec |= PFEC_implicit;
> +else if ( hvmemul_ctxt->seg_reg[x86_seg_ss].dpl == 3 )
> +pfec |= PFEC_user_mode;
> +
> +mapping = hvmemul_map_linear_addr(addr, bytes, pfec, hvmemul_ctxt);
> +if ( IS_ERR(mapping) )
> +return ~PTR_ERR(mapping);
> +
> +if ( mapping )
> +{
> +rc = x86_emul_rmw(mapping, bytes, eflags, state, ctxt);
> +hvmemul_unmap_linear_addr(mapping, addr, bytes, hvmemul_ctxt);
> +}
> +else
> +{
> +unsigned long data = 0;
> +bool_t known_gpfn = vio->mmio_access.write_access &&
> +vio->mmio_gla == (addr & PAGE_MASK);

... bool here.

Otherwise, Reviewed-by: Andrew Cooper 

> +
> +if ( bytes > sizeof(data) )
> +return X86EMUL_UNHANDLEABLE;
> +rc = hvmemul_linear_mmio_read(addr, bytes, , pfec, hvmemul_ctxt,
> +  known_gpfn);
> +if ( rc == X86EMUL_OKAY )
> +rc = x86_emul_rmw(, bytes, eflags, state, ctxt);
> +if ( rc == X86EMUL_OKAY )
> +rc = hvmemul_linear_mmio_write(addr, bytes, , pfec,
> +   hvmemul_ctxt, known_gpfn);
> +}
> +
> +return rc;
> +}
> +
>  static int hvmemul_write_discard(
>  enum x86_segment seg,
>  unsigned long offset,
> @@ -2157,6 +2212,7 @@ static const struct x86_emulate_ops hvm_
>  .read  = hvmemul_read,
>  .insn_fetch= hvmemul_insn_fetch,
>  .write = hvmemul_write,
> +.rmw   = hvmemul_rmw,
>  .cmpxchg   = hvmemul_cmpxchg,
>  .validate  = hvmemul_validate,
>  .rep_ins   = hvmemul_rep_ins,
>
>
>


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

Re: [Xen-devel] [PATCH v3 23/25] x86/HVM: make use of new read-modify-write emulator hook

[Paul Durrant]

> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: 07 December 2017 14:18
> To: xen-devel 
> Cc: Andrew Cooper ; Paul Durrant
> ; George Dunlap 
> Subject: [PATCH v3 23/25] x86/HVM: make use of new read-modify-write
> emulator hook
> 
> ..., at least as far as currently possible, i.e. when a mapping can be
> obtained.
> 
> Signed-off-by: Jan Beulich 

Reviewed-by: Paul Durrant 

> ---
> v3: New.
> 
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -1187,6 +1187,61 @@ static int hvmemul_write(
>  return X86EMUL_OKAY;
>  }
> 
> +static int hvmemul_rmw(
> +enum x86_segment seg,
> +unsigned long offset,
> +unsigned int bytes,
> +uint32_t *eflags,
> +struct x86_emulate_state *state,
> +struct x86_emulate_ctxt *ctxt)
> +{
> +struct hvm_emulate_ctxt *hvmemul_ctxt =
> +container_of(ctxt, struct hvm_emulate_ctxt, ctxt);
> +unsigned long addr, reps = 1;
> +uint32_t pfec = PFEC_page_present | PFEC_write_access;
> +struct hvm_vcpu_io *vio = >arch.hvm_vcpu.hvm_io;
> +int rc;
> +void *mapping;
> +
> +rc = hvmemul_virtual_to_linear(
> +seg, offset, bytes, , hvm_access_write, hvmemul_ctxt, );
> +if ( rc != X86EMUL_OKAY || !bytes )
> +return rc;
> +
> +if ( is_x86_system_segment(seg) )
> +pfec |= PFEC_implicit;
> +else if ( hvmemul_ctxt->seg_reg[x86_seg_ss].dpl == 3 )
> +pfec |= PFEC_user_mode;
> +
> +mapping = hvmemul_map_linear_addr(addr, bytes, pfec,
> hvmemul_ctxt);
> +if ( IS_ERR(mapping) )
> +return ~PTR_ERR(mapping);
> +
> +if ( mapping )
> +{
> +rc = x86_emul_rmw(mapping, bytes, eflags, state, ctxt);
> +hvmemul_unmap_linear_addr(mapping, addr, bytes, hvmemul_ctxt);
> +}
> +else
> +{
> +unsigned long data = 0;
> +bool_t known_gpfn = vio->mmio_access.write_access &&
> +vio->mmio_gla == (addr & PAGE_MASK);
> +
> +if ( bytes > sizeof(data) )
> +return X86EMUL_UNHANDLEABLE;
> +rc = hvmemul_linear_mmio_read(addr, bytes, , pfec,
> hvmemul_ctxt,
> +  known_gpfn);
> +if ( rc == X86EMUL_OKAY )
> +rc = x86_emul_rmw(, bytes, eflags, state, ctxt);
> +if ( rc == X86EMUL_OKAY )
> +rc = hvmemul_linear_mmio_write(addr, bytes, , pfec,
> +   hvmemul_ctxt, known_gpfn);
> +}
> +
> +return rc;
> +}
> +
>  static int hvmemul_write_discard(
>  enum x86_segment seg,
>  unsigned long offset,
> @@ -2157,6 +2212,7 @@ static const struct x86_emulate_ops hvm_
>  .read  = hvmemul_read,
>  .insn_fetch= hvmemul_insn_fetch,
>  .write = hvmemul_write,
> +.rmw   = hvmemul_rmw,
>  .cmpxchg   = hvmemul_cmpxchg,
>  .validate  = hvmemul_validate,
>  .rep_ins   = hvmemul_rep_ins,
> 
> 


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