Re: [Xen-devel] [PATCH v6 09/16] x86/hvm: limit reps to avoid the need to handle retry

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: 09 July 2015 13:04
> To: Paul Durrant
> Cc: Andrew Cooper; xen-devel@xxxxxxxxxxxxxxxxxxxx; Keir (Xen.org)
> Subject: RE: [PATCH v6 09/16] x86/hvm: limit reps to avoid the need to
> handle retry
> 
> >>> On 09.07.15 at 13:11, <Paul.Durrant@xxxxxxxxxx> wrote:
> >> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> >> Sent: 09 July 2015 11:05
> >> >>> On 03.07.15 at 18:25, <paul.durrant@xxxxxxxxxx> wrote:
> >> > @@ -287,17 +271,56 @@ static int hvmemul_do_io_addr(
> >> >      bool_t is_mmio, paddr_t addr, unsigned long *reps,
> >> >      unsigned int size, uint8_t dir, bool_t df, paddr_t ram_gpa)
> >> >  {
> >> > -    struct page_info *ram_page;
> >> > +    struct vcpu *v = current;
> >> > +    unsigned long ram_gmfn = paddr_to_pfn(ram_gpa);
> >> > +    unsigned int page_off = ram_gpa & (PAGE_SIZE - 1);
> >> > +    struct page_info *ram_page[2];
> >> > +    int nr_pages = 0;
> >>
> >> unsigned int
> >
> > No, it's intentionally signed because the unwind code at the end of the
> > function is:
> >
> >     while ( --nr_pages >= 0 )
> >         hvmemul_release_page(ram_page[nr_pages]);
> >
> > I.e. the loop terminates when nr_pages gets to -1.
> 
>      while ( nr_pages-- )
>          hvmemul_release_page(ram_page[nr_pages]);
> 

But that wouldn't be correct, since the loop would try to release ram_page[1] 
and ram_page[0] in the case where only ram_page[0] had been acquired. It would 
have to be:

      while ( nr_pages )
          hvmemul_release_page(ram_page[--nr_pages]);

which I'll do, since you so dislike signed ints.

  Paul

> Jan


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