From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Jones Subject: Re: [kvm-unit-tests PATCH v4 09/12] pci: Add generic ECAM host support Date: Sun, 12 Jun 2016 15:42:45 +0200 Message-ID: <20160612134245.l3mfhutg46k3gg4y@hawk.localdomain> References: <20160606162740.sv6frlwxku5v6xp6@hawk.localdomain> <20160611201006.GB28899@dhcp-27-118.brq.redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: kvm@vger.kernel.org, Thomas Huth To: Alexander Gordeev Return-path: Received: from mx1.redhat.com ([209.132.183.28]:46274 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752043AbcFLNmt (ORCPT ); Sun, 12 Jun 2016 09:42:49 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) (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 37DF73E50DB for ; Sun, 12 Jun 2016 13:42:49 +0000 (UTC) Content-Disposition: inline In-Reply-To: <20160611201006.GB28899@dhcp-27-118.brq.redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On Sat, Jun 11, 2016 at 10:10:07PM +0200, Alexander Gordeev wrote: > 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 ;) whatever you like. it was just a nit. > > > > + } > > > + 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; > > > +} > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html