* [PATCH] kvm/i386: replace identity_base variable with a constant
@ 2024-09-04 12:32 Paolo Bonzini
2024-09-04 16:01 ` Ani Sinha
0 siblings, 1 reply; 2+ messages in thread
From: Paolo Bonzini @ 2024-09-04 12:32 UTC (permalink / raw)
To: qemu-devel; +Cc: Ani Sinha
identity_base variable is first initialzied to address 0xfffbc000 and then
kvm_vm_set_identity_map_addr() overrides this value to address 0xfeffc000.
The initial address to which the variable was initialized was never used. Clean
everything up, placing 0xfeffc000 in a preprocessor constant.
Reported-by: Ani Sinha <anisinha@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/kvm/kvm.c | 36 ++++++++++++++++++------------------
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 599faf0ac6e..5422fd7fa7d 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -81,6 +81,16 @@
do { } while (0)
#endif
+/*
+ * On older Intel CPUs, KVM uses vm86 mode to emulate 16-bit code directly.
+ * In order to use vm86 mode, an EPT identity map and a TSS are needed.
+ * Since these must be part of guest physical memory, we need to allocate
+ * them, both by setting their start addresses in the kernel and by
+ * creating a corresponding e820 entry. We need 4 pages before the BIOS,
+ * so this value allows up to 16M BIOSes.
+ */
+#define KVM_IDENTITY_BASE 0xfeffc000
+
/* From arch/x86/kvm/lapic.h */
#define KVM_APIC_BUS_CYCLE_NS 1
#define KVM_APIC_BUS_FREQUENCY (1000000000ULL / KVM_APIC_BUS_CYCLE_NS)
@@ -3036,18 +3046,9 @@ static int kvm_vm_enable_triple_fault_event(KVMState *s)
return ret;
}
-static int kvm_vm_set_identity_map_addr(KVMState *s, uint64_t *identity_base)
+static int kvm_vm_set_identity_map_addr(KVMState *s, uint64_t identity_base)
{
- /*
- * On older Intel CPUs, KVM uses vm86 mode to emulate 16-bit code directly.
- * In order to use vm86 mode, an EPT identity map and a TSS are needed.
- * Since these must be part of guest physical memory, we need to allocate
- * them, both by setting their start addresses in the kernel and by
- * creating a corresponding e820 entry. We need 4 pages before the BIOS,
- * so this value allows up to 16M BIOSes.
- */
- *identity_base = 0xfeffc000;
- return kvm_vm_ioctl(s, KVM_SET_IDENTITY_MAP_ADDR, identity_base);
+ return kvm_vm_ioctl(s, KVM_SET_IDENTITY_MAP_ADDR, &identity_base);
}
static int kvm_vm_set_nr_mmu_pages(KVMState *s)
@@ -3064,10 +3065,9 @@ static int kvm_vm_set_nr_mmu_pages(KVMState *s)
return ret;
}
-static int kvm_vm_set_tss_addr(KVMState *s, uint64_t identity_base)
+static int kvm_vm_set_tss_addr(KVMState *s, uint64_t tss_base)
{
- /* Set TSS base one page after EPT identity map. */
- return kvm_vm_ioctl(s, KVM_SET_TSS_ADDR, identity_base);
+ return kvm_vm_ioctl(s, KVM_SET_TSS_ADDR, tss_base);
}
static int kvm_vm_enable_disable_exits(KVMState *s)
@@ -3195,7 +3195,6 @@ static void kvm_vm_enable_energy_msrs(KVMState *s)
int kvm_arch_init(MachineState *ms, KVMState *s)
{
- uint64_t identity_base = 0xfffbc000;
int ret;
struct utsname utsname;
Error *local_err = NULL;
@@ -3263,18 +3262,19 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
uname(&utsname);
lm_capable_kernel = strcmp(utsname.machine, "x86_64") == 0;
- ret = kvm_vm_set_identity_map_addr(s, &identity_base);
+ ret = kvm_vm_set_identity_map_addr(s, KVM_IDENTITY_BASE);
if (ret < 0) {
return ret;
}
- ret = kvm_vm_set_tss_addr(s, identity_base + 0x1000);
+ /* Set TSS base one page after EPT identity map. */
+ ret = kvm_vm_set_tss_addr(s, KVM_IDENTITY_BASE + 0x1000);
if (ret < 0) {
return ret;
}
/* Tell fw_cfg to notify the BIOS to reserve the range. */
- e820_add_entry(identity_base, 0x4000, E820_RESERVED);
+ e820_add_entry(KVM_IDENTITY_BASE, 0x4000, E820_RESERVED);
ret = kvm_vm_set_nr_mmu_pages(s);
if (ret < 0) {
--
2.46.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] kvm/i386: replace identity_base variable with a constant
2024-09-04 12:32 [PATCH] kvm/i386: replace identity_base variable with a constant Paolo Bonzini
@ 2024-09-04 16:01 ` Ani Sinha
0 siblings, 0 replies; 2+ messages in thread
From: Ani Sinha @ 2024-09-04 16:01 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel
> On 4 Sep 2024, at 6:02 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> identity_base variable is first initialzied to address 0xfffbc000 and then
> kvm_vm_set_identity_map_addr() overrides this value to address 0xfeffc000.
> The initial address to which the variable was initialized was never used. Clean
> everything up, placing 0xfeffc000 in a preprocessor constant.
>
> Reported-by: Ani Sinha <anisinha@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Ani Sinha <anisinha@redhat.com>
> ---
> target/i386/kvm/kvm.c | 36 ++++++++++++++++++------------------
> 1 file changed, 18 insertions(+), 18 deletions(-)
>
> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> index 599faf0ac6e..5422fd7fa7d 100644
> --- a/target/i386/kvm/kvm.c
> +++ b/target/i386/kvm/kvm.c
> @@ -81,6 +81,16 @@
> do { } while (0)
> #endif
>
> +/*
> + * On older Intel CPUs, KVM uses vm86 mode to emulate 16-bit code directly.
> + * In order to use vm86 mode, an EPT identity map and a TSS are needed.
> + * Since these must be part of guest physical memory, we need to allocate
> + * them, both by setting their start addresses in the kernel and by
> + * creating a corresponding e820 entry. We need 4 pages before the BIOS,
> + * so this value allows up to 16M BIOSes.
> + */
> +#define KVM_IDENTITY_BASE 0xfeffc000
> +
> /* From arch/x86/kvm/lapic.h */
> #define KVM_APIC_BUS_CYCLE_NS 1
> #define KVM_APIC_BUS_FREQUENCY (1000000000ULL / KVM_APIC_BUS_CYCLE_NS)
> @@ -3036,18 +3046,9 @@ static int kvm_vm_enable_triple_fault_event(KVMState *s)
> return ret;
> }
>
> -static int kvm_vm_set_identity_map_addr(KVMState *s, uint64_t *identity_base)
> +static int kvm_vm_set_identity_map_addr(KVMState *s, uint64_t identity_base)
> {
> - /*
> - * On older Intel CPUs, KVM uses vm86 mode to emulate 16-bit code directly.
> - * In order to use vm86 mode, an EPT identity map and a TSS are needed.
> - * Since these must be part of guest physical memory, we need to allocate
> - * them, both by setting their start addresses in the kernel and by
> - * creating a corresponding e820 entry. We need 4 pages before the BIOS,
> - * so this value allows up to 16M BIOSes.
> - */
> - *identity_base = 0xfeffc000;
> - return kvm_vm_ioctl(s, KVM_SET_IDENTITY_MAP_ADDR, identity_base);
> + return kvm_vm_ioctl(s, KVM_SET_IDENTITY_MAP_ADDR, &identity_base);
> }
>
> static int kvm_vm_set_nr_mmu_pages(KVMState *s)
> @@ -3064,10 +3065,9 @@ static int kvm_vm_set_nr_mmu_pages(KVMState *s)
> return ret;
> }
>
> -static int kvm_vm_set_tss_addr(KVMState *s, uint64_t identity_base)
> +static int kvm_vm_set_tss_addr(KVMState *s, uint64_t tss_base)
> {
> - /* Set TSS base one page after EPT identity map. */
> - return kvm_vm_ioctl(s, KVM_SET_TSS_ADDR, identity_base);
> + return kvm_vm_ioctl(s, KVM_SET_TSS_ADDR, tss_base);
> }
>
> static int kvm_vm_enable_disable_exits(KVMState *s)
> @@ -3195,7 +3195,6 @@ static void kvm_vm_enable_energy_msrs(KVMState *s)
>
> int kvm_arch_init(MachineState *ms, KVMState *s)
> {
> - uint64_t identity_base = 0xfffbc000;
> int ret;
> struct utsname utsname;
> Error *local_err = NULL;
> @@ -3263,18 +3262,19 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
> uname(&utsname);
> lm_capable_kernel = strcmp(utsname.machine, "x86_64") == 0;
>
> - ret = kvm_vm_set_identity_map_addr(s, &identity_base);
> + ret = kvm_vm_set_identity_map_addr(s, KVM_IDENTITY_BASE);
> if (ret < 0) {
> return ret;
> }
>
> - ret = kvm_vm_set_tss_addr(s, identity_base + 0x1000);
> + /* Set TSS base one page after EPT identity map. */
> + ret = kvm_vm_set_tss_addr(s, KVM_IDENTITY_BASE + 0x1000);
> if (ret < 0) {
> return ret;
> }
>
> /* Tell fw_cfg to notify the BIOS to reserve the range. */
> - e820_add_entry(identity_base, 0x4000, E820_RESERVED);
> + e820_add_entry(KVM_IDENTITY_BASE, 0x4000, E820_RESERVED);
>
> ret = kvm_vm_set_nr_mmu_pages(s);
> if (ret < 0) {
> --
> 2.46.0
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2024-09-04 16:03 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-04 12:32 [PATCH] kvm/i386: replace identity_base variable with a constant Paolo Bonzini
2024-09-04 16:01 ` Ani Sinha
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).