From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexander Gordeev Subject: Re: [kvm-unit-tests PATCH v4 09/12] pci: Add generic ECAM host support Date: Sat, 11 Jun 2016 22:10:07 +0200 Message-ID: <20160611201006.GB28899@dhcp-27-118.brq.redhat.com> References: <20160606162740.sv6frlwxku5v6xp6@hawk.localdomain> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: kvm@vger.kernel.org, Thomas Huth To: Andrew Jones Return-path: Received: from mx1.redhat.com ([209.132.183.28]:43218 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751823AbcFKUKM (ORCPT ); Sat, 11 Jun 2016 16:10:12 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id DCED664D18 for ; Sat, 11 Jun 2016 20:10:10 +0000 (UTC) Content-Disposition: inline In-Reply-To: <20160606162740.sv6frlwxku5v6xp6@hawk.localdomain> Sender: kvm-owner@vger.kernel.org List-ID: On Mon, Jun 06, 2016 at 06:27:40PM +0200, Andrew Jones wrote: > On Mon, Jun 06, 2016 at 02:46:38PM +0200, Alexander Gordeev wrote: > > + for (i = 0; i < nr_addr_spaces; i++) { > > + /* > > + * The PCI binding claims the numerical representation of a PCI > > + * address consists of three cells, encoded as follows: > > nit: No need to say 'claims', "The PCI binding encodes the PCI address > with three cells as follows:" > > > + * > > + * phys.hi cell: npt000ss bbbbbbbb dddddfff rrrrrrrr > > + * phys.mid cell: hhhhhhhh hhhhhhhh hhhhhhhh hhhhhhhh > > + * phys.lo cell: llllllll llllllll llllllll llllllll > > + * > > + * PCI device bus address and flags are encoded into phys.high > > + * PCI 64 bit address is encoded into phys.mid and phys.low > > + */ > > + as->type = of_flags_to_pci_type(fdt32_to_cpu(data[0])); > > + as->pci_start = ((u64)fdt32_to_cpu(data[1]) << 32) | > > + fdt32_to_cpu(data[2]); > > + > > + if (nr_range_cells == 6) { > > + as->start = fdt32_to_cpu(data[3]); > > + as->size = ((u64)fdt32_to_cpu(data[4]) << 32) | > > + fdt32_to_cpu(data[5]); > > + } else { > > + as->start = ((u64)fdt32_to_cpu(data[3]) << 32) | > > + fdt32_to_cpu(data[4]); > > + as->size = ((u64)fdt32_to_cpu(data[5]) << 32) | > > + fdt32_to_cpu(data[6]); > > + } > > + > > + data += nr_range_cells; > > + as++; (2) > > + } > > + > > + return host; > > +} > > + > > +static u64 pci_alloc_resource(u32 bar, u64 size) > > +{ > > + struct pci_host_bridge *host = pci_host_bridge; > > + struct pci_addr_space *as = &host->addr_space[0]; > > + phys_addr_t addr; > > + u32 mask; > > + int i; > > + > > + for (i = 0; i < host->nr_addr_spaces; i++) { > > + if (as->type == pci_bar_type(bar)) > > + break; > > + as++; > > nit: could have put this as++ up with the i++ like you do below Actually, I forgot to make it below (1) the same way as here :-) The reason is there are three cycles looping over address spaces and I wanted to make them alike. I chose (2) as the sample since these two assignents read better: data += nr_range_cells; as++; Will repost whatever you like, IOW ;) > > + } > > + if (i >= host->nr_addr_spaces) { > > + printf("No PCI resource found for a device\n"); > > + assert(0); > > + } > > + > > + mask = pci_bar_mask(bar); > > + size = ALIGN(size, ~mask + 1); > > + assert(as->allocated + size <= as->size); > > + > > + addr = as->pci_start + as->allocated; > > + as->allocated += size; > > + > > + return addr; > > +} > > + > > +bool pci_probe(void) > > +{ > > + pcidevaddr_t dev; > > + u8 header; > > + u32 cmd; > > + int i; > > + > > + assert(!pci_host_bridge); > > + pci_host_bridge = pci_dt_probe(); > > + if (!pci_host_bridge) > > + return false; > > + > > + for (dev = 0; dev < 256; dev++) { > > + if (!pci_dev_exists(dev)) > > + continue; > > + > > + /* We are only interested in normal PCI devices */ > > + header = pci_config_readb(dev, PCI_HEADER_TYPE); > > + if ((header & PCI_HEADER_TYPE_MASK) != PCI_HEADER_TYPE_NORMAL) > > + continue; > > + > > + cmd = PCI_COMMAND_SERR; > > + > > + for (i = 0; i < 6; i++) { > > + u64 addr, size; > > + u32 bar; > > + > > + size = pci_bar_size(dev, i); > > + if (!size) > > + break; > > + > > + bar = pci_bar_get(dev, i); > > + addr = pci_alloc_resource(bar, size); > > + pci_bar_set_addr(dev, i, addr); > > + > > + if (pci_bar_is_memory(dev, i)) > > + cmd |= PCI_COMMAND_MEMORY; > > + else > > + cmd |= PCI_COMMAND_IO; > > + > > + if (pci_bar_is64(dev, i)) > > + i++; > > + } > > + > > + pci_config_writel(dev, PCI_COMMAND, cmd); > > + } > > + > > + return true; > > +} > > + > > +/* > > + * This function is to be called from pci_translate_addr() to provide > > + * mapping between this host bridge's PCI busses address and CPU physical > > + * address. > > + */ > > +phys_addr_t pci_host_bridge_get_paddr(u64 pci_addr) > > +{ > > + struct pci_host_bridge *host = pci_host_bridge; > > + struct pci_addr_space *as = &host->addr_space[0]; > > + int i; > > + > > + for (i = 0; i < host->nr_addr_spaces; i++, as++) { (1) > > + if (pci_addr >= as->pci_start && > > + pci_addr < as->pci_start + as->size) > > + return as->start + (pci_addr - as->pci_start); > > + } > > + > > + return 0; > > +}