On Thu, Jun 09, 2016 at 10:41:02PM +0200, Alexander Gordeev wrote: > On Wed, Jun 08, 2016 at 01:59:59PM +0200, Andrew Jones wrote: > > > > So you need 5 functions > > > > > > > > uint32_t get_mask(int bar) > > > > { > > > > ... > > > > } > > > > > > > > uint32_t read_dance(int bar) > > > > { > > > > read-write1s-dance > > > > return readl > > > > } > > > > > > > > uint32_t get_size32(int bar) > > > > { > > > > uint32_t size = read_dance(bar) & get_mask(bar); > > > > return ~size + 1; > > > > } > > > > > > > > uint64_t get_size64(int bar) > > > > { > > > > uint64_t size = read_dance(bar) & get_mask(bar); > > > > uint64_t size_hi = read_dance(bar + 1); > > > > > > > > size |= size_hi << 32; > > > > return ~size + 1; > > > > } > > > > > > > > uint64_t get_size(int bar) > > > > { > > > > return bar_size(bar) == BAR64 ? get_size64(bar) : get_size32(bar); > > > > } > > > > > > If the above is a pseudo code or you want me to rework using these > > > functions? > > > > Yes and no. You can do what you like, but pci_bar_size32 should > > return a size (like my get_size32), not a 2's complement of size, > > otherwise the name is wrong. > > Right. So my pci_bar_size32 is your read_dance. Want to use that name > or i.e. pci_bar_size_complement? read-dance was more of joke name, but I am starting to like it :-) pci_bar_size_complement isn't right, as it returns an unmasked value, not the real complement. pci_bar_size_helper()? pci_bar_size_read_dance()? Whatever you like :-) > > > > If not, below is how it could look like. > > > > > > Also, looking at your get_size() I noticed a bug in my version :-) > > > when read_dance() is followed by readl() returns zero - that means no > > > (more) BAR(s). > > > > > > phys_addr_t pci_bar_size(pcidevaddr_t dev, int bar_num) > > > { > > > uint32_t size; > > > uint32_t bar; > > > > > > size = pci_bar_size32(dev, bar_num); > > > if (!size) > > > return 0; > > > > Should callers ever call pci_bar_size on a bar_num that will result > > in zero, "no more BARs"? If not, then you can assert here. Otherwise > > will all callers know what zero means, and check for it? > > Yes, it is an indication a BAR is implemented in HW. But I misled you :/ > I remember some code stopped on a zero-sized BAR, but I do not recall > what code it was. There is nothing in specs about stopping and both > Seabios and Linux just continue probing. So I will update the breaks to > continues. > > > Thanks, > > drew > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html