| Message ID | 20181210114654.31433-1-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | target/i386/kvm.c: Don't mark cpuid_data as QEMU_PACKED | expand |
On 12/10/18 12:46 PM, Peter Maydell wrote: > clang complains about taking the address of a packed > member of a struct: > > target/i386/kvm.c:1245:27: warning: taking address of packed member 'cpuid' of class or structure '' may result in an unaligned pointer value [-Waddress-of-packed-member] > c = cpuid_find_entry(&cpuid_data.cpuid, 1, 0); > ^~~~~~~~~~~~~~~~ > target/i386/kvm.c:1297:31: warning: taking address of packed member 'cpuid' of class or structure '' may result in an unaligned pointer value [-Waddress-of-packed-member] > c = cpuid_find_entry(&cpuid_data.cpuid, kvm_base, 0); > ^~~~~~~~~~~~~~~~ > > The kernel's definitions of struct kvm_cpuid2 and struct > kvm_cpuid_entry2 are carefully set up with padding fields > so that there is no between-struct padding anyway, so > the QEMU_PACKED annotation is unnecessary and might result > in the compiler generating worse code. Drop it, and instead > assert at build time that there is no stray padding. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > target/i386/kvm.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/target/i386/kvm.c b/target/i386/kvm.c > index b2401d13ea7..739cf8c8ea1 100644 > --- a/target/i386/kvm.c > +++ b/target/i386/kvm.c > @@ -864,7 +864,15 @@ int kvm_arch_init_vcpu(CPUState *cs) > struct { > struct kvm_cpuid2 cpuid; > struct kvm_cpuid_entry2 entries[KVM_MAX_CPUID_ENTRIES]; > - } QEMU_PACKED cpuid_data; > + } cpuid_data; > + /* > + * The kernel defines these structs with padding fields so there > + * should be no extra padding in our cpuid_data struct. > + */ > + QEMU_BUILD_BUG_ON(sizeof(cpuid_data) != > + sizeof(struct kvm_cpuid2) + > + sizeof(struct kvm_cpuid_entry2) * KVM_MAX_CPUID_ENTRIES); > + > X86CPU *cpu = X86_CPU(cs); > CPUX86State *env = &cpu->env; > uint32_t limit, i, j, cpuid_i; >
On 12/10/18 5:46 AM, Peter Maydell wrote: > clang complains about taking the address of a packed > member of a struct: > > target/i386/kvm.c:1245:27: warning: taking address of packed member 'cpuid' of class or structure '' may result in an unaligned pointer value [-Waddress-of-packed-member] > c = cpuid_find_entry(&cpuid_data.cpuid, 1, 0); > ^~~~~~~~~~~~~~~~ > target/i386/kvm.c:1297:31: warning: taking address of packed member 'cpuid' of class or structure '' may result in an unaligned pointer value [-Waddress-of-packed-member] > c = cpuid_find_entry(&cpuid_data.cpuid, kvm_base, 0); > ^~~~~~~~~~~~~~~~ > > The kernel's definitions of struct kvm_cpuid2 and struct > kvm_cpuid_entry2 are carefully set up with padding fields > so that there is no between-struct padding anyway, so > the QEMU_PACKED annotation is unnecessary and might result > in the compiler generating worse code. Drop it, and instead > assert at build time that there is no stray padding. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > target/i386/kvm.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
On Mon, Dec 10, 2018 at 11:46:54AM +0000, Peter Maydell wrote: > clang complains about taking the address of a packed > member of a struct: > > target/i386/kvm.c:1245:27: warning: taking address of packed member 'cpuid' of class or structure '' may result in an unaligned pointer value [-Waddress-of-packed-member] > c = cpuid_find_entry(&cpuid_data.cpuid, 1, 0); > ^~~~~~~~~~~~~~~~ > target/i386/kvm.c:1297:31: warning: taking address of packed member 'cpuid' of class or structure '' may result in an unaligned pointer value [-Waddress-of-packed-member] > c = cpuid_find_entry(&cpuid_data.cpuid, kvm_base, 0); > ^~~~~~~~~~~~~~~~ > > The kernel's definitions of struct kvm_cpuid2 and struct > kvm_cpuid_entry2 are carefully set up with padding fields > so that there is no between-struct padding anyway, so > the QEMU_PACKED annotation is unnecessary and might result > in the compiler generating worse code. Drop it, and instead > assert at build time that there is no stray padding. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Queued, thanks. -- Eduardo
diff --git a/target/i386/kvm.c b/target/i386/kvm.c index b2401d13ea7..739cf8c8ea1 100644 --- a/target/i386/kvm.c +++ b/target/i386/kvm.c @@ -864,7 +864,15 @@ int kvm_arch_init_vcpu(CPUState *cs) struct { struct kvm_cpuid2 cpuid; struct kvm_cpuid_entry2 entries[KVM_MAX_CPUID_ENTRIES]; - } QEMU_PACKED cpuid_data; + } cpuid_data; + /* + * The kernel defines these structs with padding fields so there + * should be no extra padding in our cpuid_data struct. + */ + QEMU_BUILD_BUG_ON(sizeof(cpuid_data) != + sizeof(struct kvm_cpuid2) + + sizeof(struct kvm_cpuid_entry2) * KVM_MAX_CPUID_ENTRIES); + X86CPU *cpu = X86_CPU(cs); CPUX86State *env = &cpu->env; uint32_t limit, i, j, cpuid_i;
clang complains about taking the address of a packed member of a struct: target/i386/kvm.c:1245:27: warning: taking address of packed member 'cpuid' of class or structure '' may result in an unaligned pointer value [-Waddress-of-packed-member] c = cpuid_find_entry(&cpuid_data.cpuid, 1, 0); ^~~~~~~~~~~~~~~~ target/i386/kvm.c:1297:31: warning: taking address of packed member 'cpuid' of class or structure '' may result in an unaligned pointer value [-Waddress-of-packed-member] c = cpuid_find_entry(&cpuid_data.cpuid, kvm_base, 0); ^~~~~~~~~~~~~~~~ The kernel's definitions of struct kvm_cpuid2 and struct kvm_cpuid_entry2 are carefully set up with padding fields so that there is no between-struct padding anyway, so the QEMU_PACKED annotation is unnecessary and might result in the compiler generating worse code. Drop it, and instead assert at build time that there is no stray padding. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/i386/kvm.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) -- 2.19.2