From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55571) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b2Mug-0004MU-7q for qemu-devel@nongnu.org; Mon, 16 May 2016 14:06:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b2Mub-0001Dh-KN for qemu-devel@nongnu.org; Mon, 16 May 2016 14:06:42 -0400 Received: from mout.web.de ([212.227.17.11]:50490) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b2Mub-0001DZ-A8 for qemu-devel@nongnu.org; Mon, 16 May 2016 14:06:37 -0400 References: <1462796162-13375-1-git-send-email-davidkiarie4@gmail.com> <1462796162-13375-2-git-send-email-davidkiarie4@gmail.com> From: Jan Kiszka Message-ID: <5738CE0E.7000404@web.de> Date: Sun, 15 May 2016 21:29:18 +0200 MIME-Version: 1.0 In-Reply-To: <1462796162-13375-2-git-send-email-davidkiarie4@gmail.com> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="bLk8tD3BUAHbXNp4GbG5grSe6iSn4iJX8" Subject: Re: [Qemu-devel] [V10 1/4] hw/i386: Introduce AMD IOMMU List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Kiarie , qemu-devel@nongnu.org Cc: valentine.sinitsyn@gmail.com, peterx@redhat.com, marcel@redhat.com, mst@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --bLk8tD3BUAHbXNp4GbG5grSe6iSn4iJX8 Content-Type: text/plain; charset=iso-8859-15 Content-Transfer-Encoding: quoted-printable On 2016-05-09 14:15, David Kiarie wrote: > +static void amdvi_page_walk(AMDVIAddressSpace *as, uint64_t *dte, > + IOMMUTLBEntry *ret, unsigned perms, > + hwaddr addr) > +{ > + unsigned level, present, pte_perms; > + uint64_t pte =3D dte[0], pte_addr; > + > + /* make sure the DTE has TV =3D 1 */ > + if (pte & AMDVI_DEV_TRANSLATION_VALID) { > + level =3D get_pte_translation_mode(pte); > + if (level >=3D 7) { > + AMDVI_DPRINTF(MMU, "error: translation level 0x%"PRIu8 " d= etected" > + " while translating 0x%"PRIx64, level, addr); > + return; > + } > + if (level =3D=3D 0) { > + goto no_remap; > + } > + > + while (level > 0) { > + pte_perms =3D amdvi_get_perms(pte); > + present =3D pte & 1; > + if (!present || perms !=3D (perms & pte_perms)) { > + amdvi_page_fault(as->iommu_state, as->devfn, addr, per= ms); > + AMDVI_DPRINTF(MMU, "error: page fault accessing virtua= l addr " > + "0x%"PRIx64, addr); > + return; > + } > + > + /* go to the next lower level */ > + pte_addr =3D pte & AMDVI_DEV_PT_ROOT_MASK; > + /* add offset and load pte */ > + pte_addr +=3D ((addr >> (3 + 9 * level)) & 0x1FF) << 3; > + pte =3D ldq_phys(&address_space_memory, pte_addr); > + level =3D get_pte_translation_mode(pte); > + } > + /* get access permissions from pte */ That comment is only addressing the last assignment of the followings. > + ret->iova =3D addr & AMDVI_PAGE_MASK_4K; > + ret->translated_addr =3D (pte & AMDVI_DEV_PT_ROOT_MASK) & > + AMDVI_PAGE_MASK_4K; > + ret->addr_mask =3D ~AMDVI_PAGE_MASK_4K; This does not take huge pages (2M, 1G, ...) into account. Jailhouse creates them, and its Linux guest goes mad. You need to use the correct page size here, analogously to intel_iommu.c. > + ret->perm =3D amdvi_get_perms(pte); > + return; > + } > + > +no_remap: > + ret->iova =3D addr & AMDVI_PAGE_MASK_4K; > + ret->translated_addr =3D addr & AMDVI_PAGE_MASK_4K; > + ret->addr_mask =3D ~AMDVI_PAGE_MASK_4K; > + ret->perm =3D amdvi_get_perms(pte); > + > +} > + > +/* TODO : Mark addresses as Accessed and Dirty */ > +static void amdvi_do_translate(AMDVIAddressSpace *as, hwaddr addr, > + bool is_write, IOMMUTLBEntry *ret) > +{ > + AMDVIState *s =3D as->iommu_state; > + uint16_t devid =3D PCI_DEVID(as->bus_num, as->devfn); > + AMDVIIOTLBEntry *iotlb_entry =3D amdvi_iotlb_lookup(s, addr, as->d= evfn); > + uint64_t entry[4]; > + > + if (iotlb_entry) { > + AMDVI_DPRINTF(CACHE, "hit iotlb devid: %02x:%02x.%x gpa 0x%"P= RIx64 > + " hpa 0x%"PRIx64, PCI_BUS_NUM(devid), PCI_SLOT(d= evid), > + PCI_FUNC(devid), addr, iotlb_entry->translated_a= ddr); > + ret->iova =3D addr & AMDVI_PAGE_MASK_4K; > + ret->translated_addr =3D iotlb_entry->translated_addr; > + ret->addr_mask =3D ~AMDVI_PAGE_MASK_4K; > + ret->perm =3D iotlb_entry->perms; > + return; > + } > + > + /* devices with V =3D 0 are not translated */ > + if (!amdvi_get_dte(s, devid, entry)) { > + goto out; > + } > + > + amdvi_page_walk(as, entry, ret, > + is_write ? AMDVI_PERM_WRITE : AMDVI_PERM_READ, add= r); > + > + amdvi_update_iotlb(s, as->devfn, addr, ret->translated_addr, > + ret->perm, entry[1] & AMDVI_DEV_DOMID_ID_MASK);= > + return; > + > +out: > + ret->iova =3D addr & AMDVI_PAGE_MASK_4K; > + ret->translated_addr =3D addr & AMDVI_PAGE_MASK_4K; > + ret->addr_mask =3D ~AMDVI_PAGE_MASK_4K; > + ret->perm =3D IOMMU_RW; > +} > + > +static inline bool amdvi_is_interrupt_addr(hwaddr addr) > +{ > + return addr >=3D AMDVI_INT_ADDR_FIRST && addr <=3D AMDVI_INT_ADDR_= LAST; > +} > + > +static IOMMUTLBEntry amdvi_translate(MemoryRegion *iommu, hwaddr addr,= > + bool is_write) > +{ > + AMDVI_DPRINTF(GENERAL, ""); Not a very helpful instrumentation, I would say. > + > + AMDVIAddressSpace *as =3D container_of(iommu, AMDVIAddressSpace, i= ommu); > + AMDVIState *s =3D as->iommu_state; > + IOMMUTLBEntry ret =3D { > + .target_as =3D &address_space_memory, > + .iova =3D addr, > + .translated_addr =3D 0, > + .addr_mask =3D ~(hwaddr)0, > + .perm =3D IOMMU_NONE > + }; > + > + if (!s->enabled || amdvi_is_interrupt_addr(addr)) { > + /* AMDVI disabled - corresponds to iommu=3Doff not > + * failure to provide any parameter > + */ > + ret.iova =3D addr & AMDVI_PAGE_MASK_4K; > + ret.translated_addr =3D addr & AMDVI_PAGE_MASK_4K; > + ret.addr_mask =3D ~AMDVI_PAGE_MASK_4K; > + ret.perm =3D IOMMU_RW; > + return ret; > + } > + > + amdvi_do_translate(as, addr, is_write, &ret); > + AMDVI_DPRINTF(MMU, "devid: %02x:%02x.%x gpa 0x%"PRIx64 " hpa 0x%"P= RIx64, > + as->bus_num, PCI_SLOT(as->devfn), PCI_FUNC(as->devfn= ), addr, > + ret.translated_addr); Tracing permission here in addition would be good. Jan --bLk8tD3BUAHbXNp4GbG5grSe6iSn4iJX8 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 iEYEARECAAYFAlc4ziAACgkQitSsb3rl5xR94gCg7bTP5PPso3e4hlnrUD0j23MT 6CAAoJKZeWP1J045b3bytYEpCqwBWA2o =3gK6 -----END PGP SIGNATURE----- --bLk8tD3BUAHbXNp4GbG5grSe6iSn4iJX8--