From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH 12/13] pci-assign: Generic config space access management Date: Tue, 28 Jun 2011 09:08:09 +0200 Message-ID: <4E097DD9.2080607@web.de> References: <007a0322c5a4af8a621375c4d2951eee01f2aa05.1309198794.git.jan.kiszka@web.de> <1309214919.2535.73.camel@x201> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig6684856588AEB4FD6AEA2AA2" Cc: Avi Kivity , Marcelo Tosatti , kvm@vger.kernel.org, "Michael S. Tsirkin" To: Alex Williamson Return-path: Received: from fmmailgate02.web.de ([217.72.192.227]:54397 "EHLO fmmailgate02.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755149Ab1F1HIQ (ORCPT ); Tue, 28 Jun 2011 03:08:16 -0400 In-Reply-To: <1309214919.2535.73.camel@x201> Sender: kvm-owner@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig6684856588AEB4FD6AEA2AA2 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 2011-06-28 00:48, Alex Williamson wrote: >> - case PCI_CAP_ID_VPD: >> - case PCI_CAP_ID_VNDR: >> - assigned_dev_pci_write(pci_dev, address, val, len); >> + switch (len) { >> + case 4: >> + direct_mask =3D >> + pci_get_long(assigned_dev->direct_config_write + address)= ; >> + full_access =3D 0xffffffff; >> + break; >> + case 2: >> + direct_mask =3D >> + pci_get_word(assigned_dev->direct_config_write + address)= ; >> + full_access =3D 0xffff; >> break; >> + case 1: >> + direct_mask =3D >> + pci_get_byte(assigned_dev->direct_config_write + address)= ; >> + full_access =3D 0xff; >> + break; >> + default: >> + abort(); >> + } >=20 > Shouldn't there be a if (!direct_mask) return; here? Yep. >=20 >> + if (direct_mask !=3D full_access) { >> + val &=3D direct_mask; >> + val |=3D assigned_dev_pci_read(pci_dev, address, len) & ~dire= ct_mask; >> } >> + assigned_dev_pci_write(pci_dev, address, val, len); >> } >> =20 >> static int assigned_device_pci_cap_init(PCIDevice *pci_dev) >> @@ -1404,6 +1247,10 @@ static int assigned_device_pci_cap_init(PCIDevi= ce *pci_dev) >> return ret; >> } >> =20 >> + /* direct read except for next cap pointer */ >> + memset(dev->direct_config_read + pos, 0xff, PCI_PM_SIZEOF); >> + dev->direct_config_read[pos + PCI_CAP_LIST_NEXT] =3D 0; >> + >> pmc =3D pci_get_word(pci_dev->config + pos + PCI_CAP_FLAGS); >> pmc &=3D (PCI_PM_CAP_VER_MASK | PCI_PM_CAP_DSI); >> pci_set_word(pci_dev->config + pos + PCI_CAP_FLAGS, pmc); >> @@ -1438,6 +1285,10 @@ static int assigned_device_pci_cap_init(PCIDevi= ce *pci_dev) >> return ret; >> } >> =20 >> + /* direct read except for next cap pointer */ >> + memset(dev->direct_config_read + pos, 0xff, size); >> + dev->direct_config_read[pos + PCI_CAP_LIST_NEXT] =3D 0; >> + >> type =3D pci_get_word(pci_dev->config + pos + PCI_EXP_FLAGS);= >> type =3D (type & PCI_EXP_FLAGS_TYPE) >> 8; >> if (type !=3D PCI_EXP_TYPE_ENDPOINT && >> @@ -1513,6 +1364,10 @@ static int assigned_device_pci_cap_init(PCIDevi= ce *pci_dev) >> return ret; >> } >> =20 >> + /* direct read except for next cap pointer */ >> + memset(dev->direct_config_read + pos, 0xff, 8); >> + dev->direct_config_read[pos + PCI_CAP_LIST_NEXT] =3D 0; >> + >> /* Command register, clear upper bits, including extended mod= es */ >> cmd =3D pci_get_word(pci_dev->config + pos + PCI_X_CMD); >> cmd &=3D (PCI_X_CMD_DPERR_E | PCI_X_CMD_ERO | PCI_X_CMD_MAX_R= EAD | >> @@ -1534,6 +1389,13 @@ static int assigned_device_pci_cap_init(PCIDevi= ce *pci_dev) >> if ((ret =3D pci_add_capability(pci_dev, PCI_CAP_ID_VPD, pos,= 8)) < 0) { >> return ret; >> } >> + >> + /* direct read except for next cap pointer */ >> + memset(dev->direct_config_read + pos, 0xff, 8); >> + dev->direct_config_read[pos + PCI_CAP_LIST_NEXT] =3D 0; >> + >> + /* direct write for cap content */ >> + memset(dev->direct_config_write + pos + 2, 0xff, 6); >> } >> =20 >> /* Devices can have multiple vendor capabilities, get them all */= >> @@ -1545,6 +1407,21 @@ static int assigned_device_pci_cap_init(PCIDevi= ce *pci_dev) >> pos, len)) < 0) { >> return ret; >> } >> + >> + /* direct read except for next cap pointer */ >> + memset(dev->direct_config_read + pos, 0xff, len); >> + dev->direct_config_read[pos + PCI_CAP_LIST_NEXT] =3D 0; >> + >> + /* direct write for cap content */ >> + memset(dev->direct_config_write + pos + 2, 0xff, len - 2); >> + } >> + >> + /* If real and virtual capability list status bits differ, virtua= lize the >> + * access. */ >> + if ((pci_get_word(pci_dev->config + PCI_STATUS) & PCI_STATUS_CAP_= LIST) !=3D >> + (assigned_dev_pci_read_byte(pci_dev, PCI_STATUS) & >> + PCI_STATUS_CAP_LIST)) { >> + dev->direct_config_read[PCI_STATUS] &=3D ~PCI_STATUS_CAP_LIST= ; >> } >> =20 >> return 0; >> @@ -1694,6 +1571,26 @@ static int assigned_initfn(struct PCIDevice *pc= i_dev) >> return -1; >> } >> =20 >> + /* >> + * Set up basic config space access control. Will be further refi= ned during >> + * device initialization. >> + * >> + * Direct read/write access is grangted for: >> + * - status & command registers, revision ID & class code, cache= line size, >> + * latency timer, header type, BIST >> + * - cardbus CIS pointer, subsystem vendor & ID >> + * - reserved fields >> + * - Min_Gnt & Max_Lat >> + */ >> + memset(dev->direct_config_read + PCI_COMMAND, 0xff, 12); >=20 > This is more efficient, but maybe we should memset each field for > grep-ability. Will play with that. May save some comments. >=20 > Nice change overall. Thanks, Thanks, Jan --------------enig6684856588AEB4FD6AEA2AA2 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.15 (GNU/Linux) Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org/ iEYEARECAAYFAk4Jfd0ACgkQitSsb3rl5xRC8wCg5CJtE2lbKUfXW/EgSdjB1E5Y 80MAn36uF40mXIuMrJHF26fpbRMU/Kng =1YNF -----END PGP SIGNATURE----- --------------enig6684856588AEB4FD6AEA2AA2--