Re: [Xen-devel] [RFC][PATCH 12/13] hvmloader/pci: skip reserved ranges

["Chen, Tiejun" <tiejun.chen@xxxxxxxxx>]

On 2015/4/20 22:21, Jan Beulich wrote:
> > > > On 10.04.15 at 11:22, <tiejun.chen@xxxxxxxxx> wrote:
> > --- a/tools/firmware/hvmloader/pci.c
> > +++ b/tools/firmware/hvmloader/pci.c
> > @@ -59,8 +59,8 @@ void pci_setup(void)
> >           uint32_t bar_reg;
> >           uint64_t bar_sz;
> >       } *bars = (struct bars *)scratch_start;
> > -    unsigned int i, nr_bars = 0;
> > -    uint64_t mmio_hole_size = 0;
> > +    unsigned int i, j, nr_bars = 0;
> > +    uint64_t mmio_hole_size = 0, reserved_end;
> >
> >       const char *s;
> >       /*
> > @@ -393,8 +393,23 @@ void pci_setup(void)
> >           }
> >
> >           base = (resource->base  + bar_sz - 1) & ~(uint64_t)(bar_sz - 1);
> > + reallocate_mmio:
> >           bar_data |= (uint32_t)base;
> >           bar_data_upper = (uint32_t)(base >> 32);
> > +        for ( j = 0; j < memory_map.nr_map ; j++ )
> > +        {
> > +            if ( memory_map.map[j].type != E820_RAM )
> > +            {
> > +                reserved_end = memory_map.map[j].addr + memory_map.map[j].size;
> > +                if ( check_hole_conflict(base, bar_sz,
> > +                                         memory_map.map[j].addr,
> > +                                         memory_map.map[j].size) )
> > +                {
> > +                    base = (reserved_end  + bar_sz - 1) & ~(uint64_t)(bar_sz - 
> > 1);
> > +                    goto reallocate_mmio;
> > +                }
> > +            }
> > +        }
> >           base += bar_sz;
> >
> >           if ( (base < resource->base) || (base > resource->max) )
Actually some original codes are missing here,

        if ( (base < resource->base) || (base > resource->max) )
        {
            printf("pci dev %02x:%x bar %02x size "PRIllx": no space for "
                   "resource!\n", devfn>>3, devfn&7, bar_reg,
                   PRIllx_arg(bar_sz));
            continue;
        }

I think this can guarantee the MMIO regions just fit in the available RAM.

Or am I wrong?

Thanks
Tiejun

> But you do nothing to make sure the MMIO regions all fit in the
> available window (see the code ahead of this relocating RAM if
> necessary).
>
> Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel