[Qemu-devel] [PATCH] intel_iommu: fix VTD_SID_TO_BUS
(((sid) 8) 0xff) makes no sense (((sid) 8) 0xff) seems to be what was meant. Suggested-by: Markus Armbruster arm...@redhat.com Signed-off-by: Michael S. Tsirkin m...@redhat.com --- Compile-tested only. include/hw/i386/intel_iommu.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h index f4701e1..e321ee4 100644 --- a/include/hw/i386/intel_iommu.h +++ b/include/hw/i386/intel_iommu.h @@ -37,7 +37,7 @@ #define VTD_PCI_DEVFN_MAX 256 #define VTD_PCI_SLOT(devfn) (((devfn) 3) 0x1f) #define VTD_PCI_FUNC(devfn) ((devfn) 0x07) -#define VTD_SID_TO_BUS(sid) (((sid) 8) 0xff) +#define VTD_SID_TO_BUS(sid) (((sid) 8) 0xff) #define VTD_SID_TO_DEVFN(sid) ((sid) 0xff) #define DMAR_REG_SIZE 0x230 -- MST
Re: [Qemu-devel] [PATCH] intel_iommu: fix VTD_SID_TO_BUS
Michael S. Tsirkin m...@redhat.com writes: (((sid) 8) 0xff) makes no sense (((sid) 8) 0xff) seems to be what was meant. Suggested-by: Markus Armbruster arm...@redhat.com Actually by the reporter of https://bugs.launchpad.net/bugs/1382477 Signed-off-by: Michael S. Tsirkin m...@redhat.com --- Compile-tested only. include/hw/i386/intel_iommu.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h index f4701e1..e321ee4 100644 --- a/include/hw/i386/intel_iommu.h +++ b/include/hw/i386/intel_iommu.h @@ -37,7 +37,7 @@ #define VTD_PCI_DEVFN_MAX 256 #define VTD_PCI_SLOT(devfn) (((devfn) 3) 0x1f) #define VTD_PCI_FUNC(devfn) ((devfn) 0x07) -#define VTD_SID_TO_BUS(sid) (((sid) 8) 0xff) +#define VTD_SID_TO_BUS(sid) (((sid) 8) 0xff) #define VTD_SID_TO_DEVFN(sid) ((sid) 0xff) #define DMAR_REG_SIZE 0x230 Can't find the spec right now, but it looks plausible enough. Only use is in vtd_context_device_invalidate(). Bug's impact isn't obvious to me. Reviewed-by: Markus Armbruster arm...@redhat.com
Re: [Qemu-devel] [PATCH] intel_iommu: fix VTD_SID_TO_BUS
Hi Markus, 2014-10-20 19:41 GMT+08:00 Markus Armbruster arm...@redhat.com: Michael S. Tsirkin m...@redhat.com writes: (((sid) 8) 0xff) makes no sense (((sid) 8) 0xff) seems to be what was meant. Suggested-by: Markus Armbruster arm...@redhat.com Actually by the reporter of https://bugs.launchpad.net/bugs/1382477 Signed-off-by: Michael S. Tsirkin m...@redhat.com --- Compile-tested only. include/hw/i386/intel_iommu.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h index f4701e1..e321ee4 100644 --- a/include/hw/i386/intel_iommu.h +++ b/include/hw/i386/intel_iommu.h @@ -37,7 +37,7 @@ #define VTD_PCI_DEVFN_MAX 256 #define VTD_PCI_SLOT(devfn) (((devfn) 3) 0x1f) #define VTD_PCI_FUNC(devfn) ((devfn) 0x07) -#define VTD_SID_TO_BUS(sid) (((sid) 8) 0xff) +#define VTD_SID_TO_BUS(sid) (((sid) 8) 0xff) #define VTD_SID_TO_DEVFN(sid) ((sid) 0xff) #define DMAR_REG_SIZE 0x230 Can't find the spec right now, but it looks plausible enough. Yes, this is a typo. I am sorry that I introduced such a mistake. The spec is here in Section 3.4 : http://www.intel.com/content/www/us/en/intelligent-systems/intel-technology/vt-directed-io-spec.html VTD_SID_TO_BUS(sid) is intended to be used to get the bus id from the source identifier. Thanks very much! Regards, Le Only use is in vtd_context_device_invalidate(). Bug's impact isn't obvious to me. Reviewed-by: Markus Armbruster arm...@redhat.com
Re: [Qemu-devel] [PATCH] intel_iommu: fix VTD_SID_TO_BUS
WRT intel_iommu, it does not yet seem to be as fully functional as I hoped. People also discussed the best way to handle virtio versus iommu (it bypasses it ATM). I'd like to suggest we hide the iommu from the command line help for 2.2, this way people don't activate it mistakenly. Let's make it more complete and then enable for 2.3. Thoughts? -- MST
Re: [Qemu-devel] [PATCH] intel_iommu: fix VTD_SID_TO_BUS
On 2014-10-20 16:23, Michael S. Tsirkin wrote: WRT intel_iommu, it does not yet seem to be as fully functional as I hoped. What is missing or broken? People also discussed the best way to handle virtio versus iommu (it bypasses it ATM). We need to declare in the ACPI tables that the emulated IOMMUs do not cover those special virtio devices. I'd like to suggest we hide the iommu from the command line help for 2.2, this way people don't activate it mistakenly. Let's make it more complete and then enable for 2.3. Thoughts? We need the list of issues on the table, then we can decide. Jan -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH] intel_iommu: fix VTD_SID_TO_BUS
On Mon, 2014-10-20 at 20:14 +0800, Le Tan wrote: Hi Markus, 2014-10-20 19:41 GMT+08:00 Markus Armbruster arm...@redhat.com: Michael S. Tsirkin m...@redhat.com writes: (((sid) 8) 0xff) makes no sense (((sid) 8) 0xff) seems to be what was meant. Suggested-by: Markus Armbruster arm...@redhat.com Actually by the reporter of https://bugs.launchpad.net/bugs/1382477 Signed-off-by: Michael S. Tsirkin m...@redhat.com --- Compile-tested only. include/hw/i386/intel_iommu.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h index f4701e1..e321ee4 100644 --- a/include/hw/i386/intel_iommu.h +++ b/include/hw/i386/intel_iommu.h @@ -37,7 +37,7 @@ #define VTD_PCI_DEVFN_MAX 256 #define VTD_PCI_SLOT(devfn) (((devfn) 3) 0x1f) #define VTD_PCI_FUNC(devfn) ((devfn) 0x07) -#define VTD_SID_TO_BUS(sid) (((sid) 8) 0xff) +#define VTD_SID_TO_BUS(sid) (((sid) 8) 0xff) #define VTD_SID_TO_DEVFN(sid) ((sid) 0xff) #define DMAR_REG_SIZE 0x230 Can't find the spec right now, but it looks plausible enough. Yes, this is a typo. I am sorry that I introduced such a mistake. The spec is here in Section 3.4 : http://www.intel.com/content/www/us/en/intelligent-systems/intel-technology/vt-directed-io-spec.html VTD_SID_TO_BUS(sid) is intended to be used to get the bus id from the source identifier. Thanks very much! Regards, Le Only use is in vtd_context_device_invalidate(). Bug's impact isn't obvious to me. It means that invalidation will not work as intended if a device is place on another bus than 01:xx.x (or 0 in theory) as this bus_num always evaluate to 1 or 0 as a boolean. I have been doing quite some testing of the virtual iommu, but by luck or unluck depending on viewpoint my device so far has always been in bus 1... Note that input is always supposed to be a 16 bit value here so the and is in theory not really necessary unless from a documentation and precaution point of view. Reviewed-by: Knut Omang knut.om...@oracle.com Knut Reviewed-by: Markus Armbruster arm...@redhat.com
Re: [Qemu-devel] [PATCH] intel_iommu: fix VTD_SID_TO_BUS
On Mon, 2014-10-20 at 17:23 +0300, Michael S. Tsirkin wrote: WRT intel_iommu, it does not yet seem to be as fully functional as I hoped. People also discussed the best way to handle virtio versus iommu (it bypasses it ATM). I'd like to suggest we hide the iommu from the command line help for 2.2, this way people don't activate it mistakenly. Let's make it more complete and then enable for 2.3. Thoughts? Note that you have to explicitly enable iommu support in the guest (intel_iommu=on on the boot command line in the Linux case) for it to have any effect apart from being visible in the DMAR table and logging so it should not really do any harm. From my perspective the feature works well and I have been running a few virtual machines with div.network workload stable using the additional 4 patches referred to in this post (Jan's two for interrupt remapping and two bug fixes and enhancements from me for running behind bridges) : https://lists.nongnu.org/archive/html/qemu-devel/2014-08/msg02986.html Latest version here: https://github.com/knuto/qemu/tree/sriov_patches_v2 Give me a hint and I can rebase and post the two iommu patches, I believe Jan wanted to do some more work on the interrupt remapping first. Knut
Re: [Qemu-devel] [PATCH] intel_iommu: fix VTD_SID_TO_BUS
On Mon, Oct 20, 2014 at 04:37:15PM +0200, Jan Kiszka wrote: On 2014-10-20 16:23, Michael S. Tsirkin wrote: WRT intel_iommu, it does not yet seem to be as fully functional as I hoped. What is missing or broken? PCI to PCI bridges don't work, do they? People also discussed the best way to handle virtio versus iommu (it bypasses it ATM). We need to declare in the ACPI tables that the emulated IOMMUs do not cover those special virtio devices. I'd also be fine if we flat out disallow adding virtio devices to such systems for now, or anything else that does not work. Patches for either approach aren't yet available, are they? I'd like to suggest we hide the iommu from the command line help for 2.2, this way people don't activate it mistakenly. Let's make it more complete and then enable for 2.3. Thoughts? We need the list of issues on the table, then we can decide. Jan -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH] intel_iommu: fix VTD_SID_TO_BUS
On 2014-10-20 17:15, Knut Omang wrote: On Mon, 2014-10-20 at 17:23 +0300, Michael S. Tsirkin wrote: WRT intel_iommu, it does not yet seem to be as fully functional as I hoped. People also discussed the best way to handle virtio versus iommu (it bypasses it ATM). I'd like to suggest we hide the iommu from the command line help for 2.2, this way people don't activate it mistakenly. Let's make it more complete and then enable for 2.3. Thoughts? Note that you have to explicitly enable iommu support in the guest (intel_iommu=on on the boot command line in the Linux case) for it to have any effect apart from being visible in the DMAR table and logging so it should not really do any harm. From my perspective the feature works well and I have been running a few virtual machines with div.network workload stable using the additional 4 patches referred to in this post (Jan's two for interrupt remapping and two bug fixes and enhancements from me for running behind bridges) : https://lists.nongnu.org/archive/html/qemu-devel/2014-08/msg02986.html Latest version here: https://github.com/knuto/qemu/tree/sriov_patches_v2 Give me a hint and I can rebase and post the two iommu patches, I believe Jan wanted to do some more work on the interrupt remapping first. You should avoid to depend on my series regarding upstreaming of fixes or features that can be done independently. Did your bridging fixes depend on IR? Can you break them up? Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] intel_iommu: fix VTD_SID_TO_BUS
On 2014-10-20 19:20, Michael S. Tsirkin wrote: On Mon, Oct 20, 2014 at 04:37:15PM +0200, Jan Kiszka wrote: On 2014-10-20 16:23, Michael S. Tsirkin wrote: WRT intel_iommu, it does not yet seem to be as fully functional as I hoped. What is missing or broken? PCI to PCI bridges don't work, do they? Knut had an answer, but there was something, right. People also discussed the best way to handle virtio versus iommu (it bypasses it ATM). We need to declare in the ACPI tables that the emulated IOMMUs do not cover those special virtio devices. I'd also be fine if we flat out disallow adding virtio devices to such systems for now, or anything else that does not work. I wouldn't (heavily used in my test setups). Patches for either approach aren't yet available, are they? If no one else is quicker, I'll look into the ACPI tables. Jan -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH] intel_iommu: fix VTD_SID_TO_BUS
On Mon, 2014-10-20 at 20:18 +0200, Jan Kiszka wrote: On 2014-10-20 17:15, Knut Omang wrote: On Mon, 2014-10-20 at 17:23 +0300, Michael S. Tsirkin wrote: WRT intel_iommu, it does not yet seem to be as fully functional as I hoped. People also discussed the best way to handle virtio versus iommu (it bypasses it ATM). I'd like to suggest we hide the iommu from the command line help for 2.2, this way people don't activate it mistakenly. Let's make it more complete and then enable for 2.3. Thoughts? Note that you have to explicitly enable iommu support in the guest (intel_iommu=on on the boot command line in the Linux case) for it to have any effect apart from being visible in the DMAR table and logging so it should not really do any harm. From my perspective the feature works well and I have been running a few virtual machines with div.network workload stable using the additional 4 patches referred to in this post (Jan's two for interrupt remapping and two bug fixes and enhancements from me for running behind bridges) : https://lists.nongnu.org/archive/html/qemu-devel/2014-08/msg02986.html Latest version here: https://github.com/knuto/qemu/tree/sriov_patches_v2 Give me a hint and I can rebase and post the two iommu patches, I believe Jan wanted to do some more work on the interrupt remapping first. You should avoid to depend on my series regarding upstreaming of fixes or features that can be done independently. Did your bridging fixes depend on IR? Can you break them up? Yes, that was my intention with the comment (to rebase to make my two commits independent of your interrupt remapping commits) but I realize the language in my comment was not clear - sorry for the confusion,.. Knut
Re: [Qemu-devel] [PATCH] intel_iommu: fix VTD_SID_TO_BUS
On Mon, 2014-10-20 at 21:03 +0200, Knut Omang wrote: On Mon, 2014-10-20 at 20:18 +0200, Jan Kiszka wrote: On 2014-10-20 17:15, Knut Omang wrote: On Mon, 2014-10-20 at 17:23 +0300, Michael S. Tsirkin wrote: WRT intel_iommu, it does not yet seem to be as fully functional as I hoped. People also discussed the best way to handle virtio versus iommu (it bypasses it ATM). I'd like to suggest we hide the iommu from the command line help for 2.2, this way people don't activate it mistakenly. Let's make it more complete and then enable for 2.3. Thoughts? Note that you have to explicitly enable iommu support in the guest (intel_iommu=on on the boot command line in the Linux case) for it to have any effect apart from being visible in the DMAR table and logging so it should not really do any harm. From my perspective the feature works well and I have been running a few virtual machines with div.network workload stable using the additional 4 patches referred to in this post (Jan's two for interrupt remapping and two bug fixes and enhancements from me for running behind bridges) : https://lists.nongnu.org/archive/html/qemu-devel/2014-08/msg02986.html Latest version here: https://github.com/knuto/qemu/tree/sriov_patches_v2 Sorry, I just realized this branch was for the wrong patch set - rebasing now to repost just my two iommu fixes.. Knut Give me a hint and I can rebase and post the two iommu patches, I believe Jan wanted to do some more work on the interrupt remapping first. You should avoid to depend on my series regarding upstreaming of fixes or features that can be done independently. Did your bridging fixes depend on IR? Can you break them up? Yes, that was my intention with the comment (to rebase to make my two commits independent of your interrupt remapping commits) but I realize the language in my comment was not clear - sorry for the confusion,.. Knut
