Re: [Xen-devel] [PATCH v2] x86/HVM: correct hvmemul_map_linear_addr() for multi-page case

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: 13 September 2018 12:39
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> Cc: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; xen-devel <xen-
> devel@xxxxxxxxxxxxxxxxxxxx>
> Subject: RE: [PATCH v2] x86/HVM: correct hvmemul_map_linear_addr() for
> multi-page case
> 
> >>> On 13.09.18 at 13:06, <Paul.Durrant@xxxxxxxxxx> wrote:
> >> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> >> Sent: 13 September 2018 11:13
> >>
> >> @@ -544,7 +574,7 @@ static void *hvmemul_map_linear_addr(
> >>      struct hvm_emulate_ctxt *hvmemul_ctxt)
> >>  {
> >>      struct vcpu *curr = current;
> >> -    void *err, *mapping;
> >> +    void *err = ZERO_BLOCK_PTR, *mapping;
> >
> > Given the revised logic, can't you just start *err with the value
> > ERR_PTR(~X86EMUL_OKAY) now? You can then lose the extra test in the
> first if
> > of update_map_err().
> 
> No, I don't think I can, because of ...
> 
> >>      unsigned int nr_frames = ((linear + bytes - !!bytes) >>
> PAGE_SHIFT) -
> >>          (linear >> PAGE_SHIFT) + 1;
> >>      unsigned int i;
> >> @@ -600,27 +630,28 @@ static void *hvmemul_map_linear_addr(
> >>              goto out;
> >>
> >>          case HVMTRANS_bad_gfn_to_mfn:
> >> -            err = NULL;
> >> -            goto out;
> >> +            err = update_map_err(err, NULL);
> >> +            continue;
> >>
> >>          case HVMTRANS_gfn_paged_out:
> >>          case HVMTRANS_gfn_shared:
> >> -            err = ERR_PTR(~X86EMUL_RETRY);
> >> -            goto out;
> >> +            err = update_map_err(err, ERR_PTR(~X86EMUL_RETRY));
> >> +            continue;
> >>
> >>          default:
> >> -            goto unhandleable;
> >> +            err = update_map_err(err, ERR_PTR(~X86EMUL_UNHANDLEABLE));
> >> +            continue;
> >>          }
> >>
> >>          *mfn++ = page_to_mfn(page);
> >>
> >>          if ( p2m_is_discard_write(p2mt) )
> >> -        {
> >> -            err = ERR_PTR(~X86EMUL_OKAY);
> >> -            goto out;
> >> -        }
> >> +            err = update_map_err(err, ERR_PTR(~X86EMUL_OKAY));
> 
> ... this and ...
> 
> >>      }
> >>
> >> +    if ( err != ZERO_BLOCK_PTR )
> >> +        goto out;
> 
> ... this.

Ah yes. In which case...

Reviewed-by: Paul Durrant <paul.durrant@xxxxxxxxxx>

> 
> Jan
> 


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