[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PULL 08/41] intel_iommu: support device iotlb descript
From: |
Peter Xu |
Subject: |
Re: [Qemu-devel] [PULL 08/41] intel_iommu: support device iotlb descriptor |
Date: |
Thu, 19 Jan 2017 11:28:05 +0800 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
On Thu, Jan 19, 2017 at 10:50:59AM +0800, Jason Wang wrote:
[...]
> >>+static bool vtd_process_device_iotlb_desc(IntelIOMMUState *s,
> >>+ VTDInvDesc *inv_desc)
> >>+{
> >>+ VTDAddressSpace *vtd_dev_as;
> >>+ IOMMUTLBEntry entry;
> >>+ struct VTDBus *vtd_bus;
> >>+ hwaddr addr;
> >>+ uint64_t sz;
> >>+ uint16_t sid;
> >>+ uint8_t devfn;
> >>+ bool size;
> >>+ uint8_t bus_num;
> >>+
> >>+ addr = VTD_INV_DESC_DEVICE_IOTLB_ADDR(inv_desc->hi);
> >>+ sid = VTD_INV_DESC_DEVICE_IOTLB_SID(inv_desc->lo);
> >>+ devfn = sid & 0xff;
> >>+ bus_num = sid >> 8;
> >>+ size = VTD_INV_DESC_DEVICE_IOTLB_SIZE(inv_desc->hi);
> >>+
> >>+ if ((inv_desc->lo & VTD_INV_DESC_DEVICE_IOTLB_RSVD_LO) ||
> >>+ (inv_desc->hi & VTD_INV_DESC_DEVICE_IOTLB_RSVD_HI)) {
> >>+ VTD_DPRINTF(GENERAL, "error: non-zero reserved field in Device "
> >>+ "IOTLB Invalidate Descriptor hi 0x%"PRIx64 " lo
> >>0x%"PRIx64,
> >>+ inv_desc->hi, inv_desc->lo);
> >>+ return false;
> >>+ }
> >>+
> >>+ vtd_bus = vtd_find_as_from_bus_num(s, bus_num);
> >>+ if (!vtd_bus) {
> >>+ goto done;
> >>+ }
> >>+
> >>+ vtd_dev_as = vtd_bus->dev_as[devfn];
> >>+ if (!vtd_dev_as) {
> >>+ goto done;
> >>+ }
> >>+
> >>+ if (size) {
> >>+ sz = 1 << (ctz64(~(addr | (VTD_PAGE_MASK_4K - 1))) + 1);
> >This should be 1ULL.
>
> Yes.
>
> > It could also be converted to cto64:
> >
> >(VTD_PAGE_SIZE * 2) << cto64(addr >> VTD_PAGE_SHIFT)
> >
> >Here, I'm shifting addr right to avoid the case of an addr that is all ones.
> >
> >It probably could use a comment too. :) The examples in table 2-4 of
> >the PCIe ATS specification are useful:
> >
> > S = 0, bits 15:12 = xxxx range size: 4K
> > S = 1, bits 15:12 = xxx0 range size: 8K
> > S = 1, bits 15:12 = xx01 range size: 16K
> > S = 1, bits 15:12 = x011 range size: 32K
> > S = 1, bits 15:12 = 0111 range size: 64K
> >
> > and so on
>
> This seems simpler let me add them comment and convert it to cto64.
>
> >
> >>+ addr &= ~(sz - 1);
[1]
> >>+ } else {
> >>+ sz = VTD_PAGE_SIZE;
> >>+ }
> >>+ entry.target_as = &vtd_dev_as->as;
> >>+ entry.addr_mask = sz - 1;
> >>+ entry.iova = addr;
> >If S=1, entry.iova must mask away the 1 bits that specified the size.
> >For example,
> >
> > addr = 0xabcd1000
> >
> >has cto64(0xabcd1) == 1, so it indicates a 16K invalidation from
> >0xabcd0000 to 0xabcd3fff. The "1" must be masked away with "addr & -sz"
> >or "addr & ~entry.addr_mask".
> >
> >Thanks,
> >
> >Paolo
>
> Good catch.
>
> Let me fix it.
Is above [1] doing that?
Thanks,
-- peterx
[Qemu-devel] [PULL 09/41] virtio-pci: address space translation service (ATS) support, Michael S. Tsirkin, 2017/01/10
[Qemu-devel] [PULL 10/41] acpi: add ATSR for q35, Michael S. Tsirkin, 2017/01/10
[Qemu-devel] [PULL 11/41] memory: handle alias for iommu notifier, Michael S. Tsirkin, 2017/01/10
[Qemu-devel] [PULL 12/41] memory: handle alias in memory_region_is_iommu(), Michael S. Tsirkin, 2017/01/10
[Qemu-devel] [PULL 13/41] doc/pcie: correct command line examples, Michael S. Tsirkin, 2017/01/10
[Qemu-devel] [PULL 14/41] virtio-crypto: use the correct length for cipher operation, Michael S. Tsirkin, 2017/01/10
[Qemu-devel] [PULL 15/41] cryptodev: introduce a new is_used property, Michael S. Tsirkin, 2017/01/10