On 2/23/22 23:35, Joao Martins wrote:
> On 2/23/22 21:22, Michael S. Tsirkin wrote:
>>> +static void x86_update_above_4g_mem_start(PCMachineState *pcms,
>>> + uint64_t pci_hole64_size)
>>> +{
>>> + X86MachineState *x86ms = X86_MACHINE(pcms);
>>> + uint32_t eax, vendor[3];
>>> +
>>> + host_cpuid(0x0, 0, &eax, &vendor[0], &vendor[2], &vendor[1]);
>>> + if (!IS_AMD_VENDOR(vendor)) {
>>> + return;
>>> + }
>>
>> Wait a sec, should this actually be tying things to the host CPU ID?
>> It's really about what we present to the guest though,
>> isn't it?
>
> It was the easier catch all to use cpuid without going into
> Linux UAPI specifics. But it doesn't have to tie in there, it is only
> for systems with an IOMMU present.
>
>> Also, can't we tie this to whether the AMD IOMMU is present?
>>
> I think so, I can add that. Something like a amd_iommu_exists() helper
> in util/vfio-helpers.c which checks if there's any sysfs child entries
> that start with ivhd in /sys/class/iommu/. Given that this HT region is
> hardcoded in iommu reserved regions since >=4.11 (to latest) I don't think
> it's
> even worth checking the range exists in:
>
> /sys/kernel/iommu_groups/0/reserved_regions
>
> (Also that sysfs ABI is >= 4.11 only)
Here's what I have staged in local tree, to address your comment.
Naturally the first chunk is what's affected by this patch the rest is a
precedessor patch to introduce qemu_amd_iommu_is_present(). Seems to pass
all the tests and what not.
I am not entirely sure this is the right place to put such a helper, open
to suggestions. wrt to the naming of the helper, I tried to follow the rest
of the file's style.
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index a9be5d33a291..2ea4430d5dcc 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -868,10 +868,8 @@ static void x86_update_above_4g_mem_start(PCMachineState
*pcms,
uint64_t pci_hole64_size)
{
X86MachineState *x86ms = X86_MACHINE(pcms);
- uint32_t eax, vendor[3];
- host_cpuid(0x0, 0, &eax, &vendor[0], &vendor[2], &vendor[1]);
- if (!IS_AMD_VENDOR(vendor)) {
+ if (!qemu_amd_iommu_is_present()) {
return;
}
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 7bcce3bceb0f..eb4ea071ecec 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -637,6 +637,15 @@ char *qemu_get_host_name(Error **errp);
*/
size_t qemu_get_host_physmem(void);
+/**
+ * qemu_amd_iommu_is_present:
+ *
+ * Operating system agnostic way of querying if an AMD IOMMU
+ * is present.
+ *
+ */
+bool qemu_amd_iommu_is_present(void);
+
/*
* Toggle write/execute on the pages marked MAP_JIT
* for the current thread.
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index f2be7321c59f..54cef21217c4 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -982,3 +982,32 @@ size_t qemu_get_host_physmem(void)
#endif
return 0;
}
+
+bool qemu_amd_iommu_is_present(void)
+{
+ bool found = false;
+#ifdef CONFIG_LINUX
+ struct dirent *entry;
+ char *path;
+ DIR *dir;
+
+ path = g_strdup_printf("/sys/class/iommu");
+ dir = opendir(path);
+ if (!dir) {
+ g_free(path);
+ return found;
+ }
+
+ do {
+ entry = readdir(dir);
+ if (entry && !strncmp(entry->d_name, "ivhd", 4)) {
+ found = true;
+ break;
+ }
+ } while (entry);
+
+ g_free(path);
+ closedir(dir);
+#endif
+ return found;
+}
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index af559ef3398d..c08826e7e19b 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -652,3 +652,8 @@ size_t qemu_get_host_physmem(void)
}
return 0;
}
+
+bool qemu_amd_iommu_is_present(void)
+{
+ return false;
+}
