From mboxrd@z Thu Jan 1 00:00:00 1970 From: Roger Pau =?utf-8?B?TW9ubsOp?= Subject: Re: [PATCH v4 12/19] xen: add PCI MMIO areas to memory map Date: Wed, 14 Nov 2018 13:48:28 +0100 Message-ID: <20181114124828.5syvjttcn4hrppol@mac> References: <20181102123738.16395-1-jgross@suse.com> <20181102123738.16395-13-jgross@suse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: <20181102123738.16395-13-jgross@suse.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" To: Juergen Gross Cc: hans@knorrie.org, grub-devel@gnu.org, daniel.kiper@oracle.com, xen-devel@lists.xen.org, phcoder@gmail.com List-Id: xen-devel@lists.xenproject.org T24gRnJpLCBOb3YgMDIsIDIwMTggYXQgMDE6Mzc6MzFQTSArMDEwMCwgSnVlcmdlbiBHcm9zcyB3 cm90ZToKPiBBZGQgcG9zc2libGUgUENJIHNwYWNlIE1NSU8gYXJlYXMgYXMgIlJlc2VydmVkIiB0 byB0aGUgbWVtb3J5IG1hcCBpbgo+IG9yZGVyIHRvIGF2b2lkIHVzaW5nIHRob3NlIGFyZWFzIGZv ciBzcGVjaWFsIFhlbiBwYWdlcyBsYXRlci4KClRCSCwgSSdtIG5vdCBzdXJlIHRoaXMgaXMgdGhl IGJlc3Qgd2F5IHRvIHNvbHZlIHRoZSBpc3N1ZXMgcmVsYXRlZCB0bwp3aGVyZSB0byBtYXAgc3R1 ZmYgaW4gdGhlIHBoeXNtYXAgd2l0aG91dCBjb2xsaWRpbmcgd2l0aCBlaXRoZXIKZW11bGF0ZWQg b3IgcGFzc2VkIHRocm91Z2ggTU1JTyByZWdpb25zLgoKSU1PIEkgdGhpbmsgdGhlIGd1ZXN0IHNo b3VsZCBiZSBhYmxlIHRvIHF1ZXJ5IHRoaXMgZnJvbSBYZW4sIG92ZXJhbGwgSQp3b3VsZCBkZWZl ciB0aGlzIHBhdGNoIHVudGlsIHRoZXJlJ3MgYSBkaXNjdXNzaW9uIGFib3V0IHdoZXJlIHRvIG1h cApzdHVmZiBzYWZlbHkgaW4gdGhlIHBoeXNtYXAgZm9yIGF1dG90cmFuc2xhdGVkIGd1ZXN0cy4K Cj4gU2lnbmVkLW9mZi1ieTogSnVlcmdlbiBHcm9zcyA8amdyb3NzQHN1c2UuY29tPgo+IC0tLQo+ IFY0OiBuZXcgcGF0Y2ggKFJvZ2VyIFBhdSBNb25uw6kpCj4gLS0tCj4gIGdydWItY29yZS9rZXJu L2kzODYveGVuL3B2aC5jIHwgNzAgKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysr KysrKysrKwo+ICAxIGZpbGUgY2hhbmdlZCwgNzAgaW5zZXJ0aW9ucygrKQo+IAo+IGRpZmYgLS1n aXQgYS9ncnViLWNvcmUva2Vybi9pMzg2L3hlbi9wdmguYyBiL2dydWItY29yZS9rZXJuL2kzODYv eGVuL3B2aC5jCj4gaW5kZXggNThlNmZlZmQ1Li40NDIzNTFkMWQgMTAwNjQ0Cj4gLS0tIGEvZ3J1 Yi1jb3JlL2tlcm4vaTM4Ni94ZW4vcHZoLmMKPiArKysgYi9ncnViLWNvcmUva2Vybi9pMzg2L3hl bi9wdmguYwo+IEBAIC0yMCw2ICsyMCw3IEBACj4gICNpbmNsdWRlIDxncnViL21pc2MuaD4KPiAg I2luY2x1ZGUgPGdydWIvbWVtb3J5Lmg+Cj4gICNpbmNsdWRlIDxncnViL21tLmg+Cj4gKyNpbmNs dWRlIDxncnViL3BjaS5oPgo+ICAjaW5jbHVkZSA8Z3J1Yi9pMzg2L2NwdWlkLmg+Cj4gICNpbmNs dWRlIDxncnViL2kzODYvaW8uaD4KPiAgI2luY2x1ZGUgPGdydWIveGVuLmg+Cj4gQEAgLTE3MCw2 ICsxNzEsNzMgQEAgZ3J1Yl94ZW5fc29ydF9tbWFwICh2b2lkKQo+ICAgICAgfQo+ICB9Cj4gIAo+ ICtzdGF0aWMgZ3J1Yl91aW50NjRfdAo+ICtncnViX3hlbl9wY2lfcmVhZCAoZ3J1Yl9wY2lfYWRk cmVzc190IGFkZHIsIGdydWJfdWludDMyX3QgaXNfNjRiaXQpCj4gK3sKPiArICBncnViX3VpbnQ2 NF90IHZhbDsKPiArCj4gKyAgdmFsID0gZ3J1Yl9wY2lfcmVhZCAoYWRkcik7Cj4gKyAgaWYgKGlz XzY0Yml0KQo+ICsgICAgewo+ICsgICAgICBhZGRyICs9IHNpemVvZiAoZ3J1Yl91aW50MzJfdCk7 Cj4gKyAgICAgIHZhbCB8PSAoKGdydWJfdWludDY0X3QpIGdydWJfcGNpX3JlYWQgKGFkZHIpKSA8 PCAzMjsKPiArICAgIH0KPiArCj4gKyAgcmV0dXJuIHZhbDsKPiArfQo+ICsKPiArc3RhdGljIHZv aWQKPiArZ3J1Yl94ZW5fcGNpX3dyaXRlIChncnViX3BjaV9hZGRyZXNzX3QgYWRkciwgZ3J1Yl91 aW50NjRfdCB2YWwsCj4gKwkJICAgIGdydWJfdWludDMyX3QgaXNfNjRiaXQpCj4gK3sKPiArICBn cnViX3BjaV93cml0ZSAoYWRkciwgKGdydWJfdWludDMyX3QpIHZhbCk7Cj4gKyAgaWYgKGlzXzY0 Yml0KQo+ICsgICAgewo+ICsgICAgICBhZGRyICs9IHNpemVvZiAoZ3J1Yl91aW50MzJfdCk7Cj4g KyAgICAgIGdydWJfcGNpX3dyaXRlIChhZGRyLCB2YWwgPj4gMzIpOwo+ICsgICAgfQo+ICt9Cj4g Kwo+ICtzdGF0aWMgaW50Cj4gK2dydWJfeGVuX3BjaV9tbWFwIChncnViX3BjaV9kZXZpY2VfdCBk ZXYsCj4gKwkJICAgZ3J1Yl9wY2lfaWRfdCBwY2lpZCBfX2F0dHJpYnV0ZV9fICgodW51c2VkKSks Cj4gKwkJICAgdm9pZCAqZGF0YSBfX2F0dHJpYnV0ZV9fICgodW51c2VkKSkpCj4gK3sKPiArICBp bnQgcmVnOwo+ICsgIGdydWJfcGNpX2FkZHJlc3NfdCBhZGRyOwo+ICsgIGdydWJfdWludDMyX3Qg dmFsOwo+ICsgIGdydWJfdWludDY0X3QgbW1pb19hZGRyLCBtbWlvX3NpemU7Cj4gKwo+ICsgIGlm IChucl9tYXBfZW50cmllcyA9PSBBUlJBWV9TSVpFIChtYXApKQo+ICsgICAgcmV0dXJuIDE7Cj4g Kwo+ICsgIGZvciAocmVnID0gR1JVQl9QQ0lfUkVHX0FERFJFU1NFUzsgcmVnIDwgR1JVQl9QQ0lf UkVHX0NJU19QT0lOVEVSOwo+ICsgICAgICAgcmVnICs9IHNpemVvZiAoZ3J1Yl91aW50MzJfdCkp Cj4gKyAgICB7Cj4gKyAgICAgIGFkZHIgPSBncnViX3BjaV9tYWtlX2FkZHJlc3MgKGRldiwgcmVn KTsKPiArICAgICAgdmFsID0gZ3J1Yl9wY2lfcmVhZCAoYWRkcik7Cj4gKyAgICAgIGlmICh2YWwg PT0gMCB8fAo+ICsJICAodmFsICYgR1JVQl9QQ0lfQUREUl9TUEFDRV9NQVNLKSAhPSBHUlVCX1BD SV9BRERSX1NQQUNFX01FTU9SWSkKPiArCWNvbnRpbnVlOwo+ICsKPiArICAgICAgdmFsICY9IEdS VUJfUENJX0FERFJfTUVNX1RZUEVfTUFTSzsKPiArICAgICAgbW1pb19hZGRyID0gZ3J1Yl94ZW5f cGNpX3JlYWQgKGFkZHIsIHZhbCk7Cj4gKyAgICAgIGdydWJfeGVuX3BjaV93cml0ZSAoYWRkciwg fjBVTEwsIHZhbCk7CgpZb3Ugc2hvdWxkIG1ha2Ugc3VyZSBtZW1vcnkgZGVjb2RpbmcgaXMgZGlz YWJsZWQgb24gdGhlIGNvbW1hbmQKcmVnaXN0ZXIgYmVmb3JlIHNpemluZyB0aGUgQkFScy4KCj4g KyAgICAgIG1taW9fc2l6ZSA9IH4oZ3J1Yl94ZW5fcGNpX3JlYWQgKGFkZHIsIHZhbCkgJiB+MHgw ZlVMTCkgKyAxOwoKSXNuJ3QgdGhlcmUgYSBkZWZpbmUgZm9yIHRoaXMgMHhmIHZhbHVlPwoKPiAr ICAgICAgZ3J1Yl94ZW5fcGNpX3dyaXRlIChhZGRyLCBtbWlvX2FkZHIsIHZhbCk7CgpJJ3ZlIGNv bWUgYWNyb3NzIEJBUnMgd2l0aCBzaXplIDAsIHdoaWNoIHdpbGwganVzdCB3YXN0ZSBhbiBlbnRy eSBpbgp0aGUgcGh5c21hcCBoZXJlLgoKPiArICAgICAgbWFwW25yX21hcF9lbnRyaWVzXS50eXBl ID0gR1JVQl9NRU1PUllfUkVTRVJWRUQ7Cj4gKyAgICAgIG1hcFtucl9tYXBfZW50cmllc10uYWRk ciA9IG1taW9fYWRkcjsKPiArICAgICAgbWFwW25yX21hcF9lbnRyaWVzXS5sZW4gPSBtbWlvX3Np emU7Cj4gKyAgICAgIG5yX21hcF9lbnRyaWVzKys7CgpBbHNvLCBJJ20gbm90IHN1cmUgYWJvdXQg dGhlIHNpemUgb2YgdGhlIG1hcCBhcnJheSwgYnV0IGtlZXAgaW4gbWluZApiaWcgc3lzdGVtcyBj YW4gaGF2ZSBhIGh1Z2UgYW1vdW50IG9mIFBDSSBkZXZpY2VzIChhbmQgdGh1cyBCQVJzKSBhbmQK Y291bGQgbGlrZWx5IG92ZXJydW4gdGhlIGFycmF5PwoKVGhhbmtzLCBSb2dlci4KCl9fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fClhlbi1kZXZlbCBtYWlsaW5n IGxpc3QKWGVuLWRldmVsQGxpc3RzLnhlbnByb2plY3Qub3JnCmh0dHBzOi8vbGlzdHMueGVucHJv amVjdC5vcmcvbWFpbG1hbi9saXN0aW5mby94ZW4tZGV2ZWw= From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1gMubE-0000jx-8E for mharc-grub-devel@gnu.org; Wed, 14 Nov 2018 07:48:52 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33528) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gMubB-0000hS-Iw for grub-devel@gnu.org; Wed, 14 Nov 2018 07:48:50 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gMub6-0000Np-Hy for grub-devel@gnu.org; Wed, 14 Nov 2018 07:48:49 -0500 Received: from smtp.eu.citrix.com ([185.25.65.24]:47686) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gMub4-0000Hl-HM for grub-devel@gnu.org; Wed, 14 Nov 2018 07:48:43 -0500 X-IronPort-AV: E=Sophos;i="5.56,232,1539648000"; d="scan'208";a="81843435" Date: Wed, 14 Nov 2018 13:48:28 +0100 From: Roger Pau =?utf-8?B?TW9ubsOp?= To: Juergen Gross CC: , , , , Subject: Re: [Xen-devel] [PATCH v4 12/19] xen: add PCI MMIO areas to memory map Message-ID: <20181114124828.5syvjttcn4hrppol@mac> References: <20181102123738.16395-1-jgross@suse.com> <20181102123738.16395-13-jgross@suse.com> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20181102123738.16395-13-jgross@suse.com> User-Agent: NeoMutt/20180716 X-ClientProxiedBy: AMSPEX02CAS02.citrite.net (10.69.22.113) To AMSPEX02CL02.citrite.net (10.69.22.126) X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 185.25.65.24 X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 14 Nov 2018 12:48:50 -0000 On Fri, Nov 02, 2018 at 01:37:31PM +0100, Juergen Gross wrote: > Add possible PCI space MMIO areas as "Reserved" to the memory map in > order to avoid using those areas for special Xen pages later. TBH, I'm not sure this is the best way to solve the issues related to where to map stuff in the physmap without colliding with either emulated or passed through MMIO regions. IMO I think the guest should be able to query this from Xen, overall I would defer this patch until there's a discussion about where to map stuff safely in the physmap for autotranslated guests. > Signed-off-by: Juergen Gross > --- > V4: new patch (Roger Pau Monné) > --- > grub-core/kern/i386/xen/pvh.c | 70 +++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 70 insertions(+) > > diff --git a/grub-core/kern/i386/xen/pvh.c b/grub-core/kern/i386/xen/pvh.c > index 58e6fefd5..442351d1d 100644 > --- a/grub-core/kern/i386/xen/pvh.c > +++ b/grub-core/kern/i386/xen/pvh.c > @@ -20,6 +20,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -170,6 +171,73 @@ grub_xen_sort_mmap (void) > } > } > > +static grub_uint64_t > +grub_xen_pci_read (grub_pci_address_t addr, grub_uint32_t is_64bit) > +{ > + grub_uint64_t val; > + > + val = grub_pci_read (addr); > + if (is_64bit) > + { > + addr += sizeof (grub_uint32_t); > + val |= ((grub_uint64_t) grub_pci_read (addr)) << 32; > + } > + > + return val; > +} > + > +static void > +grub_xen_pci_write (grub_pci_address_t addr, grub_uint64_t val, > + grub_uint32_t is_64bit) > +{ > + grub_pci_write (addr, (grub_uint32_t) val); > + if (is_64bit) > + { > + addr += sizeof (grub_uint32_t); > + grub_pci_write (addr, val >> 32); > + } > +} > + > +static int > +grub_xen_pci_mmap (grub_pci_device_t dev, > + grub_pci_id_t pciid __attribute__ ((unused)), > + void *data __attribute__ ((unused))) > +{ > + int reg; > + grub_pci_address_t addr; > + grub_uint32_t val; > + grub_uint64_t mmio_addr, mmio_size; > + > + if (nr_map_entries == ARRAY_SIZE (map)) > + return 1; > + > + for (reg = GRUB_PCI_REG_ADDRESSES; reg < GRUB_PCI_REG_CIS_POINTER; > + reg += sizeof (grub_uint32_t)) > + { > + addr = grub_pci_make_address (dev, reg); > + val = grub_pci_read (addr); > + if (val == 0 || > + (val & GRUB_PCI_ADDR_SPACE_MASK) != GRUB_PCI_ADDR_SPACE_MEMORY) > + continue; > + > + val &= GRUB_PCI_ADDR_MEM_TYPE_MASK; > + mmio_addr = grub_xen_pci_read (addr, val); > + grub_xen_pci_write (addr, ~0ULL, val); You should make sure memory decoding is disabled on the command register before sizing the BARs. > + mmio_size = ~(grub_xen_pci_read (addr, val) & ~0x0fULL) + 1; Isn't there a define for this 0xf value? > + grub_xen_pci_write (addr, mmio_addr, val); I've come across BARs with size 0, which will just waste an entry in the physmap here. > + map[nr_map_entries].type = GRUB_MEMORY_RESERVED; > + map[nr_map_entries].addr = mmio_addr; > + map[nr_map_entries].len = mmio_size; > + nr_map_entries++; Also, I'm not sure about the size of the map array, but keep in mind big systems can have a huge amount of PCI devices (and thus BARs) and could likely overrun the array? Thanks, Roger.