Re: [Xen-devel] [v7][RFC][PATCH 06/13] hvmloader/ram: check if guest memory is out of reserved device memory maps

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

On 2014/11/20 15:31, Jan Beulich wrote:
> > > > On 19.11.14 at 02:26, <tiejun.chen@xxxxxxxxx> wrote:
> > > > So without lookuping devices[i], how can we call func() for each sbdf as
> > > > you mentioned?
> > > You've got both rmrr and bdf in the body of for_each_rmrr_device().
> > > After all - as I said - you just open-coded it.
> > Yeah, so change this again,
> >
> > int intel_iommu_get_reserved_device_memory(iommu_grdm_t *func, void *ctxt)
> > {
> >       struct acpi_rmrr_unit *rmrr;
> >       int rc = 0;
> >       unsigned int i;
> >       u16 bdf;
> >
> >       for_each_rmrr_device ( rmrr, bdf, i )
> >       {
> >           rc = func(PFN_DOWN(rmrr->base_address),
> >                              PFN_UP(rmrr->end_address) -
> >                               PFN_DOWN(rmrr->base_address),
> >                              PCI_SBDF(rmrr->segment, bdf),
> >                             ctxt);
> >           /* Hit this entry so just go next. */
> >           if ( rc == 1 )
> >               i = rmrr->scope.devices_cnt;
> >           else if ( rc < 0 )
> >               return rc;
> >       }
> >
> >       return rc;
> > }
> Better. Another improvement would be make it not depend on the
> internal workings of for_each_rmrr_device()... And in any case you
Yes but as you see,

#define for_each_rmrr_device(rmrr, bdf, idx)            \
    list_for_each_entry(rmrr, &acpi_rmrr_units, list)   \
        /* assume there never is a bdf == 0 */          \
        for (idx = 0; (bdf = rmrr->scope.devices[idx]) && \
                 idx < rmrr->scope.devices_cnt; idx++)

So,
    for_each_rmrr_device ( rmrr, bdf, i )
    {
        rc = func(...);
        /* Hit this entry so just go next. */
        if ( rc > 0 )
            i = rmrr->scope.devices_cnt;

If you don't intend to reset something of this internal working, its 
hard to go next rmrr entry. Maybe you already have idea, so could you 
give me some hints?
> should not special case 1 - just return when rc is negative and skip
> the rest of the current RMRR when it's positive. And of course make
> the function's final return value predictable.
Like this,

        /* Hit this entry so just go next. */
        if ( rc > 0 )
            xxxx;
        else if ( rc < 0 )
            return rc;
    }

    return 0;

Thanks
Tiejun

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