From owner-freebsd-current@FreeBSD.ORG Mon Jan 12 16:29:31 2015 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 7AAB0827 for ; Mon, 12 Jan 2015 16:29:31 +0000 (UTC) Received: from mail-la0-f54.google.com (mail-la0-f54.google.com [209.85.215.54]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id E21552A2 for ; Mon, 12 Jan 2015 16:29:30 +0000 (UTC) Received: by mail-la0-f54.google.com with SMTP id pv20so24892499lab.13 for ; Mon, 12 Jan 2015 08:29:23 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=lkuS+gxNiKyMEGAKYAiOncdcGq1sPqgWuo0zLsMysI4=; b=fhG8ppj+6394t1gYkQ/O9XezYw6TZ1gpBEqORe5i1RDz36sVl2JlrYI8P5GL7ovZZz WUFejo0qHQr4MPRfYI7soQarlHpxf+yeSxswPf7uhTCEsuCOjTOK0m6mvkvUYLWADiry qdciaqn1X2OaR5OU7n3IzI4KQgRVHvB+rHok9q4cWyHF6L0weDzt2HaTrd22CLBa4qKC 03eTjqzqd2jlZ6OW9rqzNWdPx+v+pmPJMQd/2D6/sOch+reLX/AH4dOMlU0bZC9Ls6yS XChMsLbefaS75vVaBWuEikXvNveB1k4plap+AmT3R7BbwmWNRxfHwpxDZ/smGZwKtH+u q/UA== X-Gm-Message-State: ALoCoQnwSze3REPmFMamlxJV6c7A0u+6eFxJfvsHNPF3Df2YUpd6Trw8jdVqxRa9hdajbHjWWEE6 MIME-Version: 1.0 X-Received: by 10.112.55.199 with SMTP id u7mr11336887lbp.74.1421079684261; Mon, 12 Jan 2015 08:21:24 -0800 (PST) Received: by 10.25.14.208 with HTTP; Mon, 12 Jan 2015 08:21:24 -0800 (PST) In-Reply-To: <20150109172715.GU42409@kib.kiev.ua> References: <20150108203959.GR42409@kib.kiev.ua> <20150109172715.GU42409@kib.kiev.ua> Date: Mon, 12 Jan 2015 17:21:24 +0100 Message-ID: Subject: Re: [PATCH] Add support for 64-bit AHCI BAR. From: =?UTF-8?Q?Micha=C5=82_Stanek?= To: Konstantin Belousov Content-Type: multipart/mixed; boundary=001a1133fa76c74e71050c76e541 X-Content-Filtered-By: Mailman/MimeDel 2.1.18-1 Cc: freebsd-current@freebsd.org, freebsd-embedded@freebsd.org X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 12 Jan 2015 16:29:31 -0000 --001a1133fa76c74e71050c76e541 Content-Type: text/plain; charset=UTF-8 2015-01-09 18:27 GMT+01:00 Konstantin Belousov : > On Fri, Jan 09, 2015 at 06:07:39PM +0100, Micha?? Stanek wrote: > > 2015-01-08 21:40 GMT+01:00 Konstantin Belousov : > > > > > > However, if > > > > AHCI uses 64-bit base addresses, then this register consists of two > > > dwords > > > > starting at offset 0x20 - BAR4 and BAR5. This is the case on our > arm64 > > > > target and possibly other platforms using 64-bit BARs for AHCI. > > > Is it specified anywhere, or just a quirk of the specific > implementation ? > > > If it is a quirk, would it make sense to also check the vendor or > device > > > id before applying the logic ? > > > > > > > > Yes, indeed it is a quirk as I just found out that our platform vendor > > actually uses BAR(0) as AHCI ABAR, while BAR(4) is used for something > else. > > I found it implemented as a quirk in Linux AHCI code. > > The BAR is still 64-bit but in a different position than AHCI spec > stated. > > I changed it as you suggested, the new patch is in the attachment. Please > > take a look. > This is probably technically correct (I am not AHCI code author), but note > that we have more structured quirks mechanism than directly checking vendor > and device id. Look at the ahci_ids table and the quirks member. > > Add a bit declaring the need of the quirk and test the bit, instead of > the vendor/devid. > OK, I added AHCI_Q_ABAR0 as a new quirk and test the bit in ahci_pci_attach. It works the same. New patch is in the attachment. > > > > > > > > > The following patch adds a check for the extended BAR in > > > ahci_pci_attach() > > > > and sets the 'rid' in bus_alloc_resource_any accordingly. It fixes > the > > > > allocation error on our platform. > > > > > > > > Please review and test this patch on other platforms. If there are no > > > > issues then it will be committed in a week. > > > > > > > > > > From b6220884d9e71d7c4fc1c2a22ade374fc023c831 Mon Sep 17 00:00:00 2001 > > From: Michal Stanek > > Date: Fri, 9 Jan 2015 17:20:38 +0100 > > Subject: [PATCH] Add quirk for Cavium AHCI BAR location > > > > --- > > sys/dev/ahci/ahci_pci.c | 9 +++------ > > 1 file changed, 3 insertions(+), 6 deletions(-) > > > > diff --git a/sys/dev/ahci/ahci_pci.c b/sys/dev/ahci/ahci_pci.c > > index 43723a6..dce4acb 100644 > > --- a/sys/dev/ahci/ahci_pci.c > > +++ b/sys/dev/ahci/ahci_pci.c > > @@ -373,7 +373,6 @@ ahci_pci_attach(device_t dev) > > int error, i; > > uint32_t devid = pci_get_devid(dev); > > uint8_t revid = pci_get_revid(dev); > > - struct pci_map *map; > > > > i = 0; > > while (ahci_ids[i].id != 0 && > > @@ -392,11 +391,9 @@ ahci_pci_attach(device_t dev) > > ctlr->subvendorid = pci_get_subvendor(dev); > > ctlr->subdeviceid = pci_get_subdevice(dev); > > > > - /* AHCI Base Address is BAR(5) by default, unless BARs are 64-bit > */ > > - map = pci_find_bar(dev, PCIR_BAR(4)); > > - if (map != NULL && > > - ((map->pm_value & PCIM_BAR_MEM_TYPE) == PCIM_BAR_MEM_64)) > > - ctlr->r_rid = PCIR_BAR(4); > > + /* Default AHCI Base Address is BAR(5), Cavium uses BAR(0) */ > > + if (ctlr->vendorid == 0x177d && ctlr->deviceid == 0xa01c) > > + ctlr->r_rid = PCIR_BAR(0); > > else > > ctlr->r_rid = PCIR_BAR(5); > > if (!(ctlr->r_mem = bus_alloc_resource_any(dev, SYS_RES_MEMORY, > > -- > > 2.2.1 > > > > --001a1133fa76c74e71050c76e541 Content-Type: text/x-patch; charset=US-ASCII; name="0001-Add-quirk-for-Cavium-AHCI-BAR-location.patch" Content-Disposition: attachment; filename="0001-Add-quirk-for-Cavium-AHCI-BAR-location.patch" Content-Transfer-Encoding: base64 X-Attachment-Id: f_i4u1x6su0 RnJvbSA1MDA2ZTNkZmY0MzQ2ZTRhNDliNGVkYmJlMzk4Nzk5Yjk0ZGE2NTZmIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBNaWNoYWwgU3RhbmVrIDxtc3RAc2VtaWhhbGYuY29tPgpEYXRl OiBXZWQsIDcgSmFuIDIwMTUgMTg6Mjg6MTggKzAxMDAKU3ViamVjdDogW1BBVENIXSBBZGQgcXVp cmsgZm9yIENhdml1bSBBSENJIEJBUiBsb2NhdGlvbgoKUXVpcmsgYWRkZWQgdG8gb3ZlcnJpZGUg ZGVmYXVsdCBCQVIoNSkgcmlkIGZvciBBSENJLgotLS0KIHN5cy9kZXYvYWhjaS9haGNpLmggICAg IHwgMSArCiBzeXMvZGV2L2FoY2kvYWhjaV9wY2kuYyB8IDkgKysrKysrKy0tCiAyIGZpbGVzIGNo YW5nZWQsIDggaW5zZXJ0aW9ucygrKSwgMiBkZWxldGlvbnMoLSkKCmRpZmYgLS1naXQgYS9zeXMv ZGV2L2FoY2kvYWhjaS5oIGIvc3lzL2Rldi9haGNpL2FoY2kuaAppbmRleCA0NjJmMjA0Li5mZThm Yzk1IDEwMDY0NAotLS0gYS9zeXMvZGV2L2FoY2kvYWhjaS5oCisrKyBiL3N5cy9kZXYvYWhjaS9h aGNpLmgKQEAgLTU3Miw2ICs1NzIsNyBAQCBlbnVtIGFoY2lfZXJyX3R5cGUgewogI2RlZmluZSBB SENJX1FfQVRJX1BNUF9CVUcJMHgyMDAwCiAjZGVmaW5lIEFIQ0lfUV9NQVhJT182NEsJMHg0MDAw CiAjZGVmaW5lIEFIQ0lfUV9TQVRBMV9VTklUMAkweDgwMDAJCS8qIG5lZWQgYmV0dGVyIG1ldGhv ZCBmb3IgdGhpcyAqLworI2RlZmluZSBBSENJX1FfQUJBUjAJCTB4MTAwMDAKIAogI2RlZmluZSBB SENJX1FfQklUX1NUUklORwlcCiAJIlwwMjAiCQkJXApkaWZmIC0tZ2l0IGEvc3lzL2Rldi9haGNp L2FoY2lfcGNpLmMgYi9zeXMvZGV2L2FoY2kvYWhjaV9wY2kuYwppbmRleCBhZjI2OTUxLi44NTc0 ZTQ3IDEwMDY0NAotLS0gYS9zeXMvZGV2L2FoY2kvYWhjaV9wY2kuYworKysgYi9zeXMvZGV2L2Fo Y2kvYWhjaV9wY2kuYwpAQCAtMjg3LDYgKzI4Nyw3IEBAIHN0YXRpYyBjb25zdCBzdHJ1Y3Qgewog CXsweDExODQxMDM5LCAweDAwLCAiU2lTIDk2NiIsCQkwfSwKIAl7MHgxMTg1MTAzOSwgMHgwMCwg IlNpUyA5NjgiLAkJMH0sCiAJezB4MDE4NjEwMzksIDB4MDAsICJTaVMgOTY4IiwJCTB9LAorCXsw eGEwMWMxNzdkLCAweDAwLCAiVGh1bmRlclggU0FUQSIsCUFIQ0lfUV9BQkFSMH0sCiAJezB4MDAw MDAwMDAsIDB4MDAsIE5VTEwsCQkwfQogfTsKIApAQCAtMzg2LDEyICszODcsMTYgQEAgYWhjaV9w Y2lfYXR0YWNoKGRldmljZV90IGRldikKIAkgICAgcGNpX2dldF9zdWJ2ZW5kb3IoZGV2KSA9PSAw eDEwNDMgJiYKIAkgICAgcGNpX2dldF9zdWJkZXZpY2UoZGV2KSA9PSAweDgxZTQpCiAJCWN0bHIt PnF1aXJrcyB8PSBBSENJX1FfU0FUQTFfVU5JVDA7Ci0JLyogaWYgd2UgaGF2ZSBhIG1lbW9yeSBC QVIoNSkgd2UgYXJlIGxpa2VseSBvbiBhbiBBSENJIHBhcnQgKi8KIAljdGxyLT52ZW5kb3JpZCA9 IHBjaV9nZXRfdmVuZG9yKGRldik7CiAJY3Rsci0+ZGV2aWNlaWQgPSBwY2lfZ2V0X2RldmljZShk ZXYpOwogCWN0bHItPnN1YnZlbmRvcmlkID0gcGNpX2dldF9zdWJ2ZW5kb3IoZGV2KTsKIAljdGxy LT5zdWJkZXZpY2VpZCA9IHBjaV9nZXRfc3ViZGV2aWNlKGRldik7Ci0JY3Rsci0+cl9yaWQgPSBQ Q0lSX0JBUig1KTsKKworCS8qIERlZmF1bHQgQUhDSSBCYXNlIEFkZHJlc3MgaXMgQkFSKDUpLCBD YXZpdW0gdXNlcyBCQVIoMCkgKi8KKwlpZiAoY3Rsci0+cXVpcmtzICYgQUhDSV9RX0FCQVIwKQor CQljdGxyLT5yX3JpZCA9IFBDSVJfQkFSKDApOworCWVsc2UKKwkJY3Rsci0+cl9yaWQgPSBQQ0lS X0JBUig1KTsKIAlpZiAoIShjdGxyLT5yX21lbSA9IGJ1c19hbGxvY19yZXNvdXJjZV9hbnkoZGV2 LCBTWVNfUkVTX01FTU9SWSwKIAkgICAgJmN0bHItPnJfcmlkLCBSRl9BQ1RJVkUpKSkKIAkJcmV0 dXJuIEVOWElPOwotLSAKMi4yLjEKCg== --001a1133fa76c74e71050c76e541--