Re: [Xen-devel] [PATCH v3 5/8] hvmloader: Correct bug in low mmio region accounting

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Thu, 20 Jun 2013, George Dunlap wrote:
> When deciding whether to map a device in low MMIO space (<4GiB),
> hvmloader compares it with "mmio_left", which is set to the size of
> the low MMIO range (pci_mem_end - pci_mem_start).  However, even if it
> does map a device in high MMIO space, it still removes the size of its
> BAR from mmio_left.
> 
> In reality we don't need to do a separate accounting of the low memory
> available -- this can be calculated from mem_resource.  Just get rid of
> the variable and the duplicate accounting entirely.  This will make the code
> more robust.
> 
> v3:
>  - Use mem_resource values directly instead of doing duplicate accounting
> 
> Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
> CC: Ian Jackson <ian.jackson@xxxxxxxxxx>
> CC: Ian Campbell <ian.campbell@xxxxxxxxxx>
> CC: Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>
> CC: Hanweidong <hanweidong@xxxxxxxxxx>
> CC: Keir Fraser <keir@xxxxxxx>


Acked-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>

>  tools/firmware/hvmloader/pci.c |    7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/firmware/hvmloader/pci.c b/tools/firmware/hvmloader/pci.c
> index 4e8dc6a..606ccca 100644
> --- a/tools/firmware/hvmloader/pci.c
> +++ b/tools/firmware/hvmloader/pci.c
> @@ -42,7 +42,6 @@ void pci_setup(void)
>      uint32_t vga_devfn = 256;
>      uint16_t class, vendor_id, device_id;
>      unsigned int bar, pin, link, isa_irq;
> -    int64_t mmio_left;
>  
>      /* Resources assignable to PCI devices via BARs. */
>      struct resource {
> @@ -258,8 +257,6 @@ void pci_setup(void)
>      io_resource.base = 0xc000;
>      io_resource.max = 0x10000;
>  
> -    mmio_left = pci_mem_end - pci_mem_start;
> -
>      /* Assign iomem and ioport resources in descending order of size. */
>      for ( i = 0; i < nr_bars; i++ )
>      {
> @@ -267,7 +264,8 @@ void pci_setup(void)
>          bar_reg = bars[i].bar_reg;
>          bar_sz  = bars[i].bar_sz;
>  
> -        using_64bar = bars[i].is_64bar && bar64_relocate && (mmio_left < 
> bar_sz);
> +        using_64bar = bars[i].is_64bar && bar64_relocate
> +            && (bar_sz > (mem_resource.max - mem_resource.base));
>          bar_data = pci_readl(devfn, bar_reg);
>  
>          if ( (bar_data & PCI_BASE_ADDRESS_SPACE) ==
> @@ -289,7 +287,6 @@ void pci_setup(void)
>                  resource = &mem_resource;
>                  bar_data &= ~PCI_BASE_ADDRESS_MEM_MASK;
>              }
> -            mmio_left -= bar_sz;
>          }
>          else
>          {
> -- 
> 1.7.9.5
> 

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