| Message ID | 1467659769-15900-2-git-send-email-dgilbert@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Jul 04, 2016 at 08:16:04PM +0100, Dr. David Alan Gilbert (git) wrote: > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > Currently QEMU sets the x86 number of physical address bits to the > magic number 40. This is only correct on some small AMD systems; > Intel systems tend to have 36, 39, 46 bits, and large AMD systems > tend to have 48. > > Having the value different from your actual hardware is detectable > by the guest and in principal can cause problems; > The current limit of 40 stops TB VMs being created by those lucky > enough to have that much. target-i386/cpu.h has: #ifdef TARGET_X86_64 #define TARGET_PHYS_ADDR_SPACE_BITS 52 /* ??? This is really 48 bits, sign-extended, but the only thing accessible to userland with bit 48 set is the VSYSCALL, and that is handled via other mechanisms. */ #define TARGET_VIRT_ADDR_SPACE_BITS 47 #else #define TARGET_PHYS_ADDR_SPACE_BITS 36 #define TARGET_VIRT_ADDR_SPACE_BITS 32 #endif Where did those values come from? What are they used for? /* XXX: This value should match the one returned by CPUID * and in exec.c */ # if defined(TARGET_X86_64) # define PHYS_ADDR_MASK 0xffffffffffLL # else # define PHYS_ADDR_MASK 0xfffffffffLL # endif PHYS_ADDR_MASK is only used at: #define PG_HI_RSVD_MASK (PG_ADDRESS_MASK & ~PHYS_ADDR_MASK) PG_HI_RSVD_MASK is only used at x86_cpu_handle_mmu_fault(), and I don't know if that function is TCG-specific or not. > > This patch lets you set the physical bits by a cpu property but > defaults to the same existing magic 40. > > I've removed the ancient warning about the 42 bit limit in exec.c; > I can't find that limit in there and no one else seems to know where > it is. > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > --- > target-i386/cpu.c | 8 +++++--- > target-i386/cpu.h | 3 +++ > 2 files changed, 8 insertions(+), 3 deletions(-) > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index 3bd3cfc..ab13ef5 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c > @@ -2604,9 +2604,10 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, > /* virtual & phys address size in low 2 bytes. */ > /* XXX: This value must match the one used in the MMU code. */ > if (env->features[FEAT_8000_0001_EDX] & CPUID_EXT2_LM) { > - /* 64 bit processor */ > -/* XXX: The physical address space is limited to 42 bits in exec.c. */ > - *eax = 0x00003028; /* 48 bits virtual, 40 bits physical */ > + /* 64 bit processor, 48 bits virtual, configurable > + * physical bits. > + */ > + *eax = 0x00003000 + cpu->phys_bits; We need to ensure phys-bits is not out of range, either on the property setter or on realizefn. (I suggest realizefn. Custom property setters are evil) Now, related to the questions above about the cpu.h macros: what would be the appropriate range to check for? > } else { > if (env->features[FEAT_1_EDX] & CPUID_PSE36) { > *eax = 0x00000024; /* 36 bits physical */ > @@ -3257,6 +3258,7 @@ static Property x86_cpu_properties[] = { > DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, true), > DEFINE_PROP_BOOL("enforce", X86CPU, enforce_cpuid, false), > DEFINE_PROP_BOOL("kvm", X86CPU, expose_kvm, true), > + DEFINE_PROP_UINT32("phys-bits", X86CPU, phys_bits, 40), > DEFINE_PROP_UINT32("level", X86CPU, env.cpuid_level, 0), > DEFINE_PROP_UINT32("xlevel", X86CPU, env.cpuid_xlevel, 0), > DEFINE_PROP_UINT32("xlevel2", X86CPU, env.cpuid_xlevel2, 0), > diff --git a/target-i386/cpu.h b/target-i386/cpu.h > index 474b0b9..221b1a2 100644 > --- a/target-i386/cpu.h > +++ b/target-i386/cpu.h > @@ -1181,6 +1181,9 @@ struct X86CPU { > /* Compatibility bits for old machine types: */ > bool enable_cpuid_0xb; > > + /* Number of physical address bits supported */ > + uint32_t phys_bits; > + > /* in order to simplify APIC support, we leave this pointer to the > user */ > struct DeviceState *apic_state; > -- > 2.7.4 >
* Eduardo Habkost (ehabkost@redhat.com) wrote: > On Mon, Jul 04, 2016 at 08:16:04PM +0100, Dr. David Alan Gilbert (git) wrote: > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > > > Currently QEMU sets the x86 number of physical address bits to the > > magic number 40. This is only correct on some small AMD systems; > > Intel systems tend to have 36, 39, 46 bits, and large AMD systems > > tend to have 48. > > > > Having the value different from your actual hardware is detectable > > by the guest and in principal can cause problems; > > The current limit of 40 stops TB VMs being created by those lucky > > enough to have that much. > > target-i386/cpu.h has: > > #ifdef TARGET_X86_64 > #define TARGET_PHYS_ADDR_SPACE_BITS 52 > /* ??? This is really 48 bits, sign-extended, but the only thing > accessible to userland with bit 48 set is the VSYSCALL, and that > is handled via other mechanisms. */ > #define TARGET_VIRT_ADDR_SPACE_BITS 47 > #else > #define TARGET_PHYS_ADDR_SPACE_BITS 36 > #define TARGET_VIRT_ADDR_SPACE_BITS 32 > #endif > > Where did those values come from? What are they used for? > > /* XXX: This value should match the one returned by CPUID > * and in exec.c */ > # if defined(TARGET_X86_64) > # define PHYS_ADDR_MASK 0xffffffffffLL > # else > # define PHYS_ADDR_MASK 0xfffffffffLL > # endif > > PHYS_ADDR_MASK is only used at: > > #define PG_HI_RSVD_MASK (PG_ADDRESS_MASK & ~PHYS_ADDR_MASK) > > PG_HI_RSVD_MASK is only used at x86_cpu_handle_mmu_fault(), and I > don't know if that function is TCG-specific or not. Ah, so that explains what that other comment was about; to me that looks TCG only, so I should actually be careful in my phys-bits=0 case to go back to using 40 for TCG. > > This patch lets you set the physical bits by a cpu property but > > defaults to the same existing magic 40. > > > > I've removed the ancient warning about the 42 bit limit in exec.c; > > I can't find that limit in there and no one else seems to know where > > it is. > > > > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > > --- > > target-i386/cpu.c | 8 +++++--- > > target-i386/cpu.h | 3 +++ > > 2 files changed, 8 insertions(+), 3 deletions(-) > > > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > > index 3bd3cfc..ab13ef5 100644 > > --- a/target-i386/cpu.c > > +++ b/target-i386/cpu.c > > @@ -2604,9 +2604,10 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, > > /* virtual & phys address size in low 2 bytes. */ > > /* XXX: This value must match the one used in the MMU code. */ > > if (env->features[FEAT_8000_0001_EDX] & CPUID_EXT2_LM) { > > - /* 64 bit processor */ > > -/* XXX: The physical address space is limited to 42 bits in exec.c. */ > > - *eax = 0x00003028; /* 48 bits virtual, 40 bits physical */ > > + /* 64 bit processor, 48 bits virtual, configurable > > + * physical bits. > > + */ > > + *eax = 0x00003000 + cpu->phys_bits; > > We need to ensure phys-bits is not out of range, either on the > property setter or on realizefn. > > (I suggest realizefn. Custom property setters are evil) Yep I've moved the sanity check up to this patch as commented in the later one. > Now, related to the questions above about the cpu.h macros: what > would be the appropriate range to check for? Probably TARGET_PHYS_ADDR_SPACE_BITS which is the magic 52 in the x86-64 case. Dave > > } else { > > if (env->features[FEAT_1_EDX] & CPUID_PSE36) { > > *eax = 0x00000024; /* 36 bits physical */ > > @@ -3257,6 +3258,7 @@ static Property x86_cpu_properties[] = { > > DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, true), > > DEFINE_PROP_BOOL("enforce", X86CPU, enforce_cpuid, false), > > DEFINE_PROP_BOOL("kvm", X86CPU, expose_kvm, true), > > + DEFINE_PROP_UINT32("phys-bits", X86CPU, phys_bits, 40), > > DEFINE_PROP_UINT32("level", X86CPU, env.cpuid_level, 0), > > DEFINE_PROP_UINT32("xlevel", X86CPU, env.cpuid_xlevel, 0), > > DEFINE_PROP_UINT32("xlevel2", X86CPU, env.cpuid_xlevel2, 0), > > diff --git a/target-i386/cpu.h b/target-i386/cpu.h > > index 474b0b9..221b1a2 100644 > > --- a/target-i386/cpu.h > > +++ b/target-i386/cpu.h > > @@ -1181,6 +1181,9 @@ struct X86CPU { > > /* Compatibility bits for old machine types: */ > > bool enable_cpuid_0xb; > > > > + /* Number of physical address bits supported */ > > + uint32_t phys_bits; > > + > > /* in order to simplify APIC support, we leave this pointer to the > > user */ > > struct DeviceState *apic_state; > > -- > > 2.7.4 > > > > -- > Eduardo -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 3bd3cfc..ab13ef5 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -2604,9 +2604,10 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, /* virtual & phys address size in low 2 bytes. */ /* XXX: This value must match the one used in the MMU code. */ if (env->features[FEAT_8000_0001_EDX] & CPUID_EXT2_LM) { - /* 64 bit processor */ -/* XXX: The physical address space is limited to 42 bits in exec.c. */ - *eax = 0x00003028; /* 48 bits virtual, 40 bits physical */ + /* 64 bit processor, 48 bits virtual, configurable + * physical bits. + */ + *eax = 0x00003000 + cpu->phys_bits; } else { if (env->features[FEAT_1_EDX] & CPUID_PSE36) { *eax = 0x00000024; /* 36 bits physical */ @@ -3257,6 +3258,7 @@ static Property x86_cpu_properties[] = { DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, true), DEFINE_PROP_BOOL("enforce", X86CPU, enforce_cpuid, false), DEFINE_PROP_BOOL("kvm", X86CPU, expose_kvm, true), + DEFINE_PROP_UINT32("phys-bits", X86CPU, phys_bits, 40), DEFINE_PROP_UINT32("level", X86CPU, env.cpuid_level, 0), DEFINE_PROP_UINT32("xlevel", X86CPU, env.cpuid_xlevel, 0), DEFINE_PROP_UINT32("xlevel2", X86CPU, env.cpuid_xlevel2, 0), diff --git a/target-i386/cpu.h b/target-i386/cpu.h index 474b0b9..221b1a2 100644 --- a/target-i386/cpu.h +++ b/target-i386/cpu.h @@ -1181,6 +1181,9 @@ struct X86CPU { /* Compatibility bits for old machine types: */ bool enable_cpuid_0xb; + /* Number of physical address bits supported */ + uint32_t phys_bits; + /* in order to simplify APIC support, we leave this pointer to the user */ struct DeviceState *apic_state;