* [kvm-unit-tests PATCH 0/3] Patches for clang compilation
@ 2019-10-10 18:35 Bill Wendling
2019-10-10 18:35 ` [kvm-unit-tests PATCH 1/3] x86: emulator: use "SSE2" for the target Bill Wendling
` (4 more replies)
0 siblings, 5 replies; 37+ messages in thread
From: Bill Wendling @ 2019-10-10 18:35 UTC (permalink / raw)
To: kvm, pbonzini; +Cc: jmattson, Bill Wendling
These three patches fixes some compilation issues that clang's
encountering. I ran the tests and there are no regressions with gcc.
Bill Wendling (3):
x86: emulator: use "SSE2" for the target
pci: use uint64_t for unsigned long values
Makefile: use "-Werror" in cc-option
Makefile | 19 +++++++++++--------
lib/pci.c | 18 +++++++++---------
lib/pci.h | 4 ++--
x86/emulator.c | 2 +-
4 files changed, 23 insertions(+), 20 deletions(-)
--
2.23.0.700.g56cf767bdb-goog
^ permalink raw reply [flat|nested] 37+ messages in thread
* [kvm-unit-tests PATCH 1/3] x86: emulator: use "SSE2" for the target
2019-10-10 18:35 [kvm-unit-tests PATCH 0/3] Patches for clang compilation Bill Wendling
@ 2019-10-10 18:35 ` Bill Wendling
2019-10-10 18:35 ` [kvm-unit-tests PATCH 2/3] pci: use uint64_t for unsigned long values Bill Wendling
` (3 subsequent siblings)
4 siblings, 0 replies; 37+ messages in thread
From: Bill Wendling @ 2019-10-10 18:35 UTC (permalink / raw)
To: kvm, pbonzini; +Cc: jmattson, Bill Wendling
The movdqu and movapd instructions are SSE2 instructions. Clang
interprets the __attribute__((target("sse"))) as allowing SSE only
instructions. Using SSE2 instructions cause an error.
Signed-off-by: Bill Wendling <morbo@google.com>
---
x86/emulator.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/x86/emulator.c b/x86/emulator.c
index 621caf9..bec0154 100644
--- a/x86/emulator.c
+++ b/x86/emulator.c
@@ -657,7 +657,7 @@ static bool sseeq(sse_union *v1, sse_union *v2)
return ok;
}
-static __attribute__((target("sse"))) void test_sse(sse_union *mem)
+static __attribute__((target("sse2"))) void test_sse(sse_union *mem)
{
sse_union v;
--
2.23.0.700.g56cf767bdb-goog
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [kvm-unit-tests PATCH 2/3] pci: use uint64_t for unsigned long values
2019-10-10 18:35 [kvm-unit-tests PATCH 0/3] Patches for clang compilation Bill Wendling
2019-10-10 18:35 ` [kvm-unit-tests PATCH 1/3] x86: emulator: use "SSE2" for the target Bill Wendling
@ 2019-10-10 18:35 ` Bill Wendling
2019-10-11 9:12 ` Alexandru Elisei
2019-10-10 18:35 ` [kvm-unit-tests PATCH 3/3] Makefile: use "-Werror" in cc-option Bill Wendling
` (2 subsequent siblings)
4 siblings, 1 reply; 37+ messages in thread
From: Bill Wendling @ 2019-10-10 18:35 UTC (permalink / raw)
To: kvm, pbonzini; +Cc: jmattson, Bill Wendling
The "pci_bar_*" functions work with unsigned long values, but were using
uint32_t for the data types. Clang complains about this. So we bump up
the type to uint64_t.
lib/pci.c:110:3: error: implicit conversion from 'unsigned long' to 'uint32_t' (aka 'unsigned int') changes value from 18446744073709551612 to 4294967292 [-Werror,-Wconstant-conversion]
PCI_BASE_ADDRESS_IO_MASK : PCI_BASE_ADDRESS_MEM_MASK;
^~~~~~~~~~~~~~~~~~~~~~~~
/usr/local/google/home/morbo/kvm-unit-tests/lib/linux/pci_regs.h:100:36: note: expanded from macro 'PCI_BASE_ADDRESS_IO_MASK'
#define PCI_BASE_ADDRESS_IO_MASK (~0x03UL)
^~~~~~~
lib/pci.c:110:30: error: implicit conversion from 'unsigned long' to 'uint32_t' (aka 'unsigned int') changes value from 18446744073709551600 to 4294967280 [-Werror,-Wconstant-conversion]
PCI_BASE_ADDRESS_IO_MASK : PCI_BASE_ADDRESS_MEM_MASK;
^~~~~~~~~~~~~~~~~~~~~~~~~
/usr/local/google/home/morbo/kvm-unit-tests/lib/linux/pci_regs.h:99:37: note: expanded from macro 'PCI_BASE_ADDRESS_MEM_MASK'
#define PCI_BASE_ADDRESS_MEM_MASK (~0x0fUL)
^~~~~~~
Signed-off-by: Bill Wendling <morbo@google.com>
---
lib/pci.c | 18 +++++++++---------
lib/pci.h | 4 ++--
2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/lib/pci.c b/lib/pci.c
index daa33e1..e554209 100644
--- a/lib/pci.c
+++ b/lib/pci.c
@@ -104,13 +104,13 @@ pcidevaddr_t pci_find_dev(uint16_t vendor_id, uint16_t device_id)
return PCIDEVADDR_INVALID;
}
-uint32_t pci_bar_mask(uint32_t bar)
+uint64_t pci_bar_mask(uint32_t bar)
{
return (bar & PCI_BASE_ADDRESS_SPACE_IO) ?
PCI_BASE_ADDRESS_IO_MASK : PCI_BASE_ADDRESS_MEM_MASK;
}
-uint32_t pci_bar_get(struct pci_dev *dev, int bar_num)
+uint64_t pci_bar_get(struct pci_dev *dev, int bar_num)
{
ASSERT_BAR_NUM(bar_num);
@@ -120,13 +120,13 @@ uint32_t pci_bar_get(struct pci_dev *dev, int bar_num)
static phys_addr_t __pci_bar_get_addr(struct pci_dev *dev, int bar_num)
{
- uint32_t bar = pci_bar_get(dev, bar_num);
- uint32_t mask = pci_bar_mask(bar);
+ uint64_t bar = pci_bar_get(dev, bar_num);
+ uint64_t mask = pci_bar_mask(bar);
uint64_t addr = bar & mask;
phys_addr_t phys_addr;
if (pci_bar_is64(dev, bar_num))
- addr |= (uint64_t)pci_bar_get(dev, bar_num + 1) << 32;
+ addr |= pci_bar_get(dev, bar_num + 1) << 32;
phys_addr = pci_translate_addr(dev->bdf, addr);
assert(phys_addr != INVALID_PHYS_ADDR);
@@ -189,7 +189,7 @@ static uint32_t pci_bar_size_helper(struct pci_dev *dev, int bar_num)
phys_addr_t pci_bar_size(struct pci_dev *dev, int bar_num)
{
- uint32_t bar, size;
+ uint64_t bar, size;
size = pci_bar_size_helper(dev, bar_num);
if (!size)
@@ -210,7 +210,7 @@ phys_addr_t pci_bar_size(struct pci_dev *dev, int bar_num)
bool pci_bar_is_memory(struct pci_dev *dev, int bar_num)
{
- uint32_t bar = pci_bar_get(dev, bar_num);
+ uint64_t bar = pci_bar_get(dev, bar_num);
return !(bar & PCI_BASE_ADDRESS_SPACE_IO);
}
@@ -222,7 +222,7 @@ bool pci_bar_is_valid(struct pci_dev *dev, int bar_num)
bool pci_bar_is64(struct pci_dev *dev, int bar_num)
{
- uint32_t bar = pci_bar_get(dev, bar_num);
+ uint64_t bar = pci_bar_get(dev, bar_num);
if (bar & PCI_BASE_ADDRESS_SPACE_IO)
return false;
@@ -234,7 +234,7 @@ bool pci_bar_is64(struct pci_dev *dev, int bar_num)
void pci_bar_print(struct pci_dev *dev, int bar_num)
{
phys_addr_t size, start, end;
- uint32_t bar;
+ uint64_t bar;
if (!pci_bar_is_valid(dev, bar_num))
return;
diff --git a/lib/pci.h b/lib/pci.h
index 689f03c..cd12938 100644
--- a/lib/pci.h
+++ b/lib/pci.h
@@ -60,8 +60,8 @@ extern pcidevaddr_t pci_find_dev(uint16_t vendor_id, uint16_t device_id);
extern phys_addr_t pci_bar_get_addr(struct pci_dev *dev, int bar_num);
extern void pci_bar_set_addr(struct pci_dev *dev, int bar_num, phys_addr_t addr);
extern phys_addr_t pci_bar_size(struct pci_dev *dev, int bar_num);
-extern uint32_t pci_bar_get(struct pci_dev *dev, int bar_num);
-extern uint32_t pci_bar_mask(uint32_t bar);
+extern uint64_t pci_bar_get(struct pci_dev *dev, int bar_num);
+extern uint64_t pci_bar_mask(uint32_t bar);
extern bool pci_bar_is64(struct pci_dev *dev, int bar_num);
extern bool pci_bar_is_memory(struct pci_dev *dev, int bar_num);
extern bool pci_bar_is_valid(struct pci_dev *dev, int bar_num);
--
2.23.0.700.g56cf767bdb-goog
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [kvm-unit-tests PATCH 3/3] Makefile: use "-Werror" in cc-option
2019-10-10 18:35 [kvm-unit-tests PATCH 0/3] Patches for clang compilation Bill Wendling
2019-10-10 18:35 ` [kvm-unit-tests PATCH 1/3] x86: emulator: use "SSE2" for the target Bill Wendling
2019-10-10 18:35 ` [kvm-unit-tests PATCH 2/3] pci: use uint64_t for unsigned long values Bill Wendling
@ 2019-10-10 18:35 ` Bill Wendling
2019-10-15 7:29 ` Thomas Huth
2019-10-14 19:24 ` [kvm-unit-tests PATCH 0/4] Patches for clang compilation Bill Wendling
2019-10-15 0:04 ` [kvm-unit-tests PATCH v2 " Bill Wendling
4 siblings, 1 reply; 37+ messages in thread
From: Bill Wendling @ 2019-10-10 18:35 UTC (permalink / raw)
To: kvm, pbonzini; +Cc: jmattson, Bill Wendling
The "cc-option" macro should use "-Werror" to determine if a flag is
supported. Otherwise the test may not return a nonzero result. Also
conditionalize some of the warning flags which aren't supported by
clang.
Signed-off-by: Bill Wendling <morbo@google.com>
---
Makefile | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/Makefile b/Makefile
index 32414dc..3ec0458 100644
--- a/Makefile
+++ b/Makefile
@@ -46,30 +46,33 @@ include $(SRCDIR)/$(TEST_DIR)/Makefile
# cc-option
# Usage: OP_CFLAGS+=$(call cc-option, -falign-functions=0, -malign-functions=0)
-cc-option = $(shell if $(CC) $(1) -S -o /dev/null -xc /dev/null \
+cc-option = $(shell if $(CC) -Werror $(1) -S -o /dev/null -xc /dev/null \
> /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;)
COMMON_CFLAGS += -g $(autodepend-flags)
-COMMON_CFLAGS += -Wall -Wwrite-strings -Wclobbered -Wempty-body -Wuninitialized
-COMMON_CFLAGS += -Wignored-qualifiers -Wunused-but-set-parameter
-COMMON_CFLAGS += -Werror
+COMMON_CFLAGS += -Wall -Wwrite-strings -Wempty-body -Wuninitialized
+COMMON_CFLAGS += -Wignored-qualifiers -Werror
+
frame-pointer-flag=-f$(if $(KEEP_FRAME_POINTER),no-,)omit-frame-pointer
fomit_frame_pointer := $(call cc-option, $(frame-pointer-flag), "")
fnostack_protector := $(call cc-option, -fno-stack-protector, "")
fnostack_protector_all := $(call cc-option, -fno-stack-protector-all, "")
-wno_frame_address := $(call cc-option, -Wno-frame-address, "")
fno_pic := $(call cc-option, -fno-pic, "")
no_pie := $(call cc-option, -no-pie, "")
COMMON_CFLAGS += $(fomit_frame_pointer)
COMMON_CFLAGS += $(fno_stack_protector)
COMMON_CFLAGS += $(fno_stack_protector_all)
-COMMON_CFLAGS += $(wno_frame_address)
COMMON_CFLAGS += $(if $(U32_LONG_FMT),-D__U32_LONG_FMT__,)
COMMON_CFLAGS += $(fno_pic) $(no_pie)
+COMMON_CFLAGS += $(call cc-option, -Wno-frame-address, "")
+COMMON_CFLAGS += $(call cc-option, -Wclobbered, "")
+COMMON_CFLAGS += $(call cc-option, -Wunused-but-set-parameter, "")
+COMMON_CFLAGS += $(call cc-option, -Wmissing-parameter-type, "")
+COMMON_CFLAGS += $(call cc-option, -Wold-style-declaration, "")
+
CFLAGS += $(COMMON_CFLAGS)
-CFLAGS += -Wmissing-parameter-type -Wold-style-declaration -Woverride-init
-CFLAGS += -Wmissing-prototypes -Wstrict-prototypes
+CFLAGS += -Woverride-init -Wmissing-prototypes -Wstrict-prototypes
CXXFLAGS += $(COMMON_CFLAGS)
--
2.23.0.700.g56cf767bdb-goog
^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [kvm-unit-tests PATCH 2/3] pci: use uint64_t for unsigned long values
2019-10-10 18:35 ` [kvm-unit-tests PATCH 2/3] pci: use uint64_t for unsigned long values Bill Wendling
@ 2019-10-11 9:12 ` Alexandru Elisei
2019-10-12 8:26 ` [kvm-unit-tests PATCH 1/1] " Bill Wendling
0 siblings, 1 reply; 37+ messages in thread
From: Alexandru Elisei @ 2019-10-11 9:12 UTC (permalink / raw)
To: Bill Wendling, kvm, pbonzini; +Cc: jmattson
Hi,
On 10/10/19 7:35 PM, Bill Wendling wrote:
> The "pci_bar_*" functions work with unsigned long values, but were using
> uint32_t for the data types. Clang complains about this. So we bump up
> the type to uint64_t.
Your patch might fix the warning, but the bar functions were returning 32 bits
because the bars are 32 bits and because they are being read using functions
that return 32 bit values.
>
> lib/pci.c:110:3: error: implicit conversion from 'unsigned long' to 'uint32_t' (aka 'unsigned int') changes value from 18446744073709551612 to 4294967292 [-Werror,-Wconstant-conversion]
> PCI_BASE_ADDRESS_IO_MASK : PCI_BASE_ADDRESS_MEM_MASK;
> ^~~~~~~~~~~~~~~~~~~~~~~~
> /usr/local/google/home/morbo/kvm-unit-tests/lib/linux/pci_regs.h:100:36: note: expanded from macro 'PCI_BASE_ADDRESS_IO_MASK'
> #define PCI_BASE_ADDRESS_IO_MASK (~0x03UL)
> ^~~~~~~
> lib/pci.c:110:30: error: implicit conversion from 'unsigned long' to 'uint32_t' (aka 'unsigned int') changes value from 18446744073709551600 to 4294967280 [-Werror,-Wconstant-conversion]
> PCI_BASE_ADDRESS_IO_MASK : PCI_BASE_ADDRESS_MEM_MASK;
> ^~~~~~~~~~~~~~~~~~~~~~~~~
> /usr/local/google/home/morbo/kvm-unit-tests/lib/linux/pci_regs.h:99:37: note: expanded from macro 'PCI_BASE_ADDRESS_MEM_MASK'
> #define PCI_BASE_ADDRESS_MEM_MASK (~0x0fUL)
I think the issue is that the mask should be 32 bits, but unsigned long is 64
bits on your architecture. I think it's safe to use ~0x0fU here, because we're
only interested in the least significant 4 bits.
Regards,
Alex
> ^~~~~~~
>
> Signed-off-by: Bill Wendling <morbo@google.com>
> ---
> lib/pci.c | 18 +++++++++---------
> lib/pci.h | 4 ++--
> 2 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/lib/pci.c b/lib/pci.c
> index daa33e1..e554209 100644
> --- a/lib/pci.c
> +++ b/lib/pci.c
> @@ -104,13 +104,13 @@ pcidevaddr_t pci_find_dev(uint16_t vendor_id, uint16_t device_id)
> return PCIDEVADDR_INVALID;
> }
>
> -uint32_t pci_bar_mask(uint32_t bar)
> +uint64_t pci_bar_mask(uint32_t bar)
> {
> return (bar & PCI_BASE_ADDRESS_SPACE_IO) ?
> PCI_BASE_ADDRESS_IO_MASK : PCI_BASE_ADDRESS_MEM_MASK;
> }
>
> -uint32_t pci_bar_get(struct pci_dev *dev, int bar_num)
> +uint64_t pci_bar_get(struct pci_dev *dev, int bar_num)
> {
> ASSERT_BAR_NUM(bar_num);
>
> @@ -120,13 +120,13 @@ uint32_t pci_bar_get(struct pci_dev *dev, int bar_num)
>
> static phys_addr_t __pci_bar_get_addr(struct pci_dev *dev, int bar_num)
> {
> - uint32_t bar = pci_bar_get(dev, bar_num);
> - uint32_t mask = pci_bar_mask(bar);
> + uint64_t bar = pci_bar_get(dev, bar_num);
> + uint64_t mask = pci_bar_mask(bar);
> uint64_t addr = bar & mask;
> phys_addr_t phys_addr;
>
> if (pci_bar_is64(dev, bar_num))
> - addr |= (uint64_t)pci_bar_get(dev, bar_num + 1) << 32;
> + addr |= pci_bar_get(dev, bar_num + 1) << 32;
>
> phys_addr = pci_translate_addr(dev->bdf, addr);
> assert(phys_addr != INVALID_PHYS_ADDR);
> @@ -189,7 +189,7 @@ static uint32_t pci_bar_size_helper(struct pci_dev *dev, int bar_num)
>
> phys_addr_t pci_bar_size(struct pci_dev *dev, int bar_num)
> {
> - uint32_t bar, size;
> + uint64_t bar, size;
>
> size = pci_bar_size_helper(dev, bar_num);
> if (!size)
> @@ -210,7 +210,7 @@ phys_addr_t pci_bar_size(struct pci_dev *dev, int bar_num)
>
> bool pci_bar_is_memory(struct pci_dev *dev, int bar_num)
> {
> - uint32_t bar = pci_bar_get(dev, bar_num);
> + uint64_t bar = pci_bar_get(dev, bar_num);
>
> return !(bar & PCI_BASE_ADDRESS_SPACE_IO);
> }
> @@ -222,7 +222,7 @@ bool pci_bar_is_valid(struct pci_dev *dev, int bar_num)
>
> bool pci_bar_is64(struct pci_dev *dev, int bar_num)
> {
> - uint32_t bar = pci_bar_get(dev, bar_num);
> + uint64_t bar = pci_bar_get(dev, bar_num);
>
> if (bar & PCI_BASE_ADDRESS_SPACE_IO)
> return false;
> @@ -234,7 +234,7 @@ bool pci_bar_is64(struct pci_dev *dev, int bar_num)
> void pci_bar_print(struct pci_dev *dev, int bar_num)
> {
> phys_addr_t size, start, end;
> - uint32_t bar;
> + uint64_t bar;
>
> if (!pci_bar_is_valid(dev, bar_num))
> return;
> diff --git a/lib/pci.h b/lib/pci.h
> index 689f03c..cd12938 100644
> --- a/lib/pci.h
> +++ b/lib/pci.h
> @@ -60,8 +60,8 @@ extern pcidevaddr_t pci_find_dev(uint16_t vendor_id, uint16_t device_id);
> extern phys_addr_t pci_bar_get_addr(struct pci_dev *dev, int bar_num);
> extern void pci_bar_set_addr(struct pci_dev *dev, int bar_num, phys_addr_t addr);
> extern phys_addr_t pci_bar_size(struct pci_dev *dev, int bar_num);
> -extern uint32_t pci_bar_get(struct pci_dev *dev, int bar_num);
> -extern uint32_t pci_bar_mask(uint32_t bar);
> +extern uint64_t pci_bar_get(struct pci_dev *dev, int bar_num);
> +extern uint64_t pci_bar_mask(uint32_t bar);
> extern bool pci_bar_is64(struct pci_dev *dev, int bar_num);
> extern bool pci_bar_is_memory(struct pci_dev *dev, int bar_num);
> extern bool pci_bar_is_valid(struct pci_dev *dev, int bar_num);
^ permalink raw reply [flat|nested] 37+ messages in thread
* [kvm-unit-tests PATCH 1/1] pci: use uint64_t for unsigned long values
2019-10-11 9:12 ` Alexandru Elisei
@ 2019-10-12 8:26 ` Bill Wendling
2019-10-14 16:33 ` Alexandru Elisei
2019-10-14 16:36 ` Sean Christopherson
0 siblings, 2 replies; 37+ messages in thread
From: Bill Wendling @ 2019-10-12 8:26 UTC (permalink / raw)
To: kvm, pbonzini, alexandru.elisei; +Cc: jmattson, Bill Wendling
The "pci_bar_*" functions use 64-bit masks, but the results are assigned
to 32-bit variables. Use 32-bit masks, since we're interested only in
the least significant 4-bits.
Signed-off-by: Bill Wendling <morbo@google.com>
---
lib/linux/pci_regs.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/linux/pci_regs.h b/lib/linux/pci_regs.h
index 1becea8..3bc2b92 100644
--- a/lib/linux/pci_regs.h
+++ b/lib/linux/pci_regs.h
@@ -96,8 +96,8 @@
#define PCI_BASE_ADDRESS_MEM_TYPE_1M 0x02 /* Below 1M [obsolete] */
#define PCI_BASE_ADDRESS_MEM_TYPE_64 0x04 /* 64 bit address */
#define PCI_BASE_ADDRESS_MEM_PREFETCH 0x08 /* prefetchable? */
-#define PCI_BASE_ADDRESS_MEM_MASK (~0x0fUL)
-#define PCI_BASE_ADDRESS_IO_MASK (~0x03UL)
+#define PCI_BASE_ADDRESS_MEM_MASK (~0x0fU)
+#define PCI_BASE_ADDRESS_IO_MASK (~0x03U)
/* bit 1 is reserved if address_space = 1 */
/* Header type 0 (normal devices) */
--
2.23.0.700.g56cf767bdb-goog
^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [kvm-unit-tests PATCH 1/1] pci: use uint64_t for unsigned long values
2019-10-12 8:26 ` [kvm-unit-tests PATCH 1/1] " Bill Wendling
@ 2019-10-14 16:33 ` Alexandru Elisei
2019-10-14 16:36 ` Sean Christopherson
1 sibling, 0 replies; 37+ messages in thread
From: Alexandru Elisei @ 2019-10-14 16:33 UTC (permalink / raw)
To: Bill Wendling, kvm, pbonzini; +Cc: jmattson
Hi,
On 10/12/19 9:26 AM, Bill Wendling wrote:
> The "pci_bar_*" functions use 64-bit masks, but the results are assigned
> to 32-bit variables. Use 32-bit masks, since we're interested only in
> the least significant 4-bits.
>
> Signed-off-by: Bill Wendling <morbo@google.com>
> ---
> lib/linux/pci_regs.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
In my first comment I didn't realize that the file is actually a copy of a Linux
user API header file (it's taken from include/uapi/linux/pci_regs.h). I don't
think relying on changes to our own copy of the file is the best idea, because we
might forget to include them the next time we sync our version with Linux.
I think the best approach would be to do the explicit cast to uint32_t in
pci_bar_mask.
Thanks,
Alex
>
> diff --git a/lib/linux/pci_regs.h b/lib/linux/pci_regs.h
> index 1becea8..3bc2b92 100644
> --- a/lib/linux/pci_regs.h
> +++ b/lib/linux/pci_regs.h
> @@ -96,8 +96,8 @@
> #define PCI_BASE_ADDRESS_MEM_TYPE_1M 0x02 /* Below 1M [obsolete] */
> #define PCI_BASE_ADDRESS_MEM_TYPE_64 0x04 /* 64 bit address */
> #define PCI_BASE_ADDRESS_MEM_PREFETCH 0x08 /* prefetchable? */
> -#define PCI_BASE_ADDRESS_MEM_MASK (~0x0fUL)
> -#define PCI_BASE_ADDRESS_IO_MASK (~0x03UL)
> +#define PCI_BASE_ADDRESS_MEM_MASK (~0x0fU)
> +#define PCI_BASE_ADDRESS_IO_MASK (~0x03U)
> /* bit 1 is reserved if address_space = 1 */
>
> /* Header type 0 (normal devices) */
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [kvm-unit-tests PATCH 1/1] pci: use uint64_t for unsigned long values
2019-10-12 8:26 ` [kvm-unit-tests PATCH 1/1] " Bill Wendling
2019-10-14 16:33 ` Alexandru Elisei
@ 2019-10-14 16:36 ` Sean Christopherson
1 sibling, 0 replies; 37+ messages in thread
From: Sean Christopherson @ 2019-10-14 16:36 UTC (permalink / raw)
To: Bill Wendling; +Cc: kvm, pbonzini, alexandru.elisei, jmattson
The subject "pci: use uint64_t for unsigned long values" does not reflect
what the patch does.
This should be tagged v2.
Generally speaking, resend the whole series as v2 (or v3, etc...) even if
only one patch is being tweaked. It's fairly obvious in this case that
the other two other patches from v1 are still relevant, but that won't
always be true.
On Sat, Oct 12, 2019 at 01:26:23AM -0700, Bill Wendling wrote:
> The "pci_bar_*" functions use 64-bit masks, but the results are assigned
> to 32-bit variables. Use 32-bit masks, since we're interested only in
> the least significant 4-bits.
>
> Signed-off-by: Bill Wendling <morbo@google.com>
> ---
> lib/linux/pci_regs.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/lib/linux/pci_regs.h b/lib/linux/pci_regs.h
> index 1becea8..3bc2b92 100644
> --- a/lib/linux/pci_regs.h
> +++ b/lib/linux/pci_regs.h
> @@ -96,8 +96,8 @@
> #define PCI_BASE_ADDRESS_MEM_TYPE_1M 0x02 /* Below 1M [obsolete] */
> #define PCI_BASE_ADDRESS_MEM_TYPE_64 0x04 /* 64 bit address */
> #define PCI_BASE_ADDRESS_MEM_PREFETCH 0x08 /* prefetchable? */
> -#define PCI_BASE_ADDRESS_MEM_MASK (~0x0fUL)
> -#define PCI_BASE_ADDRESS_IO_MASK (~0x03UL)
> +#define PCI_BASE_ADDRESS_MEM_MASK (~0x0fU)
> +#define PCI_BASE_ADDRESS_IO_MASK (~0x03U)
The defines themselves should still be 64-bit, e.g. there's a flag right
above to denote a 64-bit address. Explicitly casting the return value in
pci_bar_mask() where we're dealing with 32-bit addresses would be
preferable.
> /* bit 1 is reserved if address_space = 1 */
>
> /* Header type 0 (normal devices) */
> --
> 2.23.0.700.g56cf767bdb-goog
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* [kvm-unit-tests PATCH 0/4] Patches for clang compilation
2019-10-10 18:35 [kvm-unit-tests PATCH 0/3] Patches for clang compilation Bill Wendling
` (2 preceding siblings ...)
2019-10-10 18:35 ` [kvm-unit-tests PATCH 3/3] Makefile: use "-Werror" in cc-option Bill Wendling
@ 2019-10-14 19:24 ` Bill Wendling
2019-10-14 19:24 ` [kvm-unit-tests PATCH 1/4] x86: emulator: use "SSE2" for the target Bill Wendling
` (4 more replies)
2019-10-15 0:04 ` [kvm-unit-tests PATCH v2 " Bill Wendling
4 siblings, 5 replies; 37+ messages in thread
From: Bill Wendling @ 2019-10-14 19:24 UTC (permalink / raw)
To: kvm, pbonzini; +Cc: jmattson, sean.j.christopherson, Bill Wendling
These four patches fix compilation issues that clang's encountering.
I ran the tests and there are no regressions with gcc.
Bill Wendling (4):
x86: emulator: use "SSE2" for the target
pci: cast the masks to the appropriate size
Makefile: use "-Werror" in cc-option
Makefile: add "cxx-option" for C++ builds
Makefile | 23 +++++++++++++++--------
lib/pci.c | 3 ++-
x86/emulator.c | 2 +-
3 files changed, 18 insertions(+), 10 deletions(-)
--
2.23.0.700.g56cf767bdb-goog
^ permalink raw reply [flat|nested] 37+ messages in thread
* [kvm-unit-tests PATCH 1/4] x86: emulator: use "SSE2" for the target
2019-10-14 19:24 ` [kvm-unit-tests PATCH 0/4] Patches for clang compilation Bill Wendling
@ 2019-10-14 19:24 ` Bill Wendling
2019-10-14 19:24 ` [kvm-unit-tests PATCH 2/4] pci: cast the masks to the appropriate size Bill Wendling
` (3 subsequent siblings)
4 siblings, 0 replies; 37+ messages in thread
From: Bill Wendling @ 2019-10-14 19:24 UTC (permalink / raw)
To: kvm, pbonzini; +Cc: jmattson, sean.j.christopherson, Bill Wendling
The movdqu and movapd instructions are SSE2 instructions. Clang
interprets the __attribute__((target("sse"))) as allowing SSE only
instructions. Using SSE2 instructions cause an error.
Signed-off-by: Bill Wendling <morbo@google.com>
---
x86/emulator.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/x86/emulator.c b/x86/emulator.c
index 621caf9..bec0154 100644
--- a/x86/emulator.c
+++ b/x86/emulator.c
@@ -657,7 +657,7 @@ static bool sseeq(sse_union *v1, sse_union *v2)
return ok;
}
-static __attribute__((target("sse"))) void test_sse(sse_union *mem)
+static __attribute__((target("sse2"))) void test_sse(sse_union *mem)
{
sse_union v;
--
2.23.0.700.g56cf767bdb-goog
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [kvm-unit-tests PATCH 2/4] pci: cast the masks to the appropriate size
2019-10-14 19:24 ` [kvm-unit-tests PATCH 0/4] Patches for clang compilation Bill Wendling
2019-10-14 19:24 ` [kvm-unit-tests PATCH 1/4] x86: emulator: use "SSE2" for the target Bill Wendling
@ 2019-10-14 19:24 ` Bill Wendling
2019-10-14 19:24 ` [kvm-unit-tests PATCH 3/4] Makefile: use "-Werror" in cc-option Bill Wendling
` (2 subsequent siblings)
4 siblings, 0 replies; 37+ messages in thread
From: Bill Wendling @ 2019-10-14 19:24 UTC (permalink / raw)
To: kvm, pbonzini; +Cc: jmattson, sean.j.christopherson, Bill Wendling
At this point, we're dealing with 32-bit addresses, therefore downcast
the masks to 32-bits.
Signed-off-by: Bill Wendling <morbo@google.com>
---
lib/pci.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/pci.c b/lib/pci.c
index daa33e1..1b85411 100644
--- a/lib/pci.c
+++ b/lib/pci.c
@@ -107,7 +107,8 @@ pcidevaddr_t pci_find_dev(uint16_t vendor_id, uint16_t device_id)
uint32_t pci_bar_mask(uint32_t bar)
{
return (bar & PCI_BASE_ADDRESS_SPACE_IO) ?
- PCI_BASE_ADDRESS_IO_MASK : PCI_BASE_ADDRESS_MEM_MASK;
+ (uint32_t)PCI_BASE_ADDRESS_IO_MASK :
+ (uint32_t)PCI_BASE_ADDRESS_MEM_MASK;
}
uint32_t pci_bar_get(struct pci_dev *dev, int bar_num)
--
2.23.0.700.g56cf767bdb-goog
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [kvm-unit-tests PATCH 3/4] Makefile: use "-Werror" in cc-option
2019-10-14 19:24 ` [kvm-unit-tests PATCH 0/4] Patches for clang compilation Bill Wendling
2019-10-14 19:24 ` [kvm-unit-tests PATCH 1/4] x86: emulator: use "SSE2" for the target Bill Wendling
2019-10-14 19:24 ` [kvm-unit-tests PATCH 2/4] pci: cast the masks to the appropriate size Bill Wendling
@ 2019-10-14 19:24 ` Bill Wendling
2019-10-14 19:24 ` [kvm-unit-tests PATCH 4/4] Makefile: add "cxx-option" for C++ builds Bill Wendling
2019-10-14 19:25 ` [kvm-unit-tests PATCH 0/4] Patches for clang compilation Bill Wendling
4 siblings, 0 replies; 37+ messages in thread
From: Bill Wendling @ 2019-10-14 19:24 UTC (permalink / raw)
To: kvm, pbonzini; +Cc: jmattson, sean.j.christopherson, Bill Wendling
The "cc-option" macro should use "-Werror" to determine if a flag is
supported. Otherwise the test may not return a nonzero result. Also
conditionalize some of the warning flags which aren't supported by
clang.
Signed-off-by: Bill Wendling <morbo@google.com>
---
Makefile | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/Makefile b/Makefile
index 32414dc..3ec0458 100644
--- a/Makefile
+++ b/Makefile
@@ -46,30 +46,33 @@ include $(SRCDIR)/$(TEST_DIR)/Makefile
# cc-option
# Usage: OP_CFLAGS+=$(call cc-option, -falign-functions=0, -malign-functions=0)
-cc-option = $(shell if $(CC) $(1) -S -o /dev/null -xc /dev/null \
+cc-option = $(shell if $(CC) -Werror $(1) -S -o /dev/null -xc /dev/null \
> /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;)
COMMON_CFLAGS += -g $(autodepend-flags)
-COMMON_CFLAGS += -Wall -Wwrite-strings -Wclobbered -Wempty-body -Wuninitialized
-COMMON_CFLAGS += -Wignored-qualifiers -Wunused-but-set-parameter
-COMMON_CFLAGS += -Werror
+COMMON_CFLAGS += -Wall -Wwrite-strings -Wempty-body -Wuninitialized
+COMMON_CFLAGS += -Wignored-qualifiers -Werror
+
frame-pointer-flag=-f$(if $(KEEP_FRAME_POINTER),no-,)omit-frame-pointer
fomit_frame_pointer := $(call cc-option, $(frame-pointer-flag), "")
fnostack_protector := $(call cc-option, -fno-stack-protector, "")
fnostack_protector_all := $(call cc-option, -fno-stack-protector-all, "")
-wno_frame_address := $(call cc-option, -Wno-frame-address, "")
fno_pic := $(call cc-option, -fno-pic, "")
no_pie := $(call cc-option, -no-pie, "")
COMMON_CFLAGS += $(fomit_frame_pointer)
COMMON_CFLAGS += $(fno_stack_protector)
COMMON_CFLAGS += $(fno_stack_protector_all)
-COMMON_CFLAGS += $(wno_frame_address)
COMMON_CFLAGS += $(if $(U32_LONG_FMT),-D__U32_LONG_FMT__,)
COMMON_CFLAGS += $(fno_pic) $(no_pie)
+COMMON_CFLAGS += $(call cc-option, -Wno-frame-address, "")
+COMMON_CFLAGS += $(call cc-option, -Wclobbered, "")
+COMMON_CFLAGS += $(call cc-option, -Wunused-but-set-parameter, "")
+COMMON_CFLAGS += $(call cc-option, -Wmissing-parameter-type, "")
+COMMON_CFLAGS += $(call cc-option, -Wold-style-declaration, "")
+
CFLAGS += $(COMMON_CFLAGS)
-CFLAGS += -Wmissing-parameter-type -Wold-style-declaration -Woverride-init
-CFLAGS += -Wmissing-prototypes -Wstrict-prototypes
+CFLAGS += -Woverride-init -Wmissing-prototypes -Wstrict-prototypes
CXXFLAGS += $(COMMON_CFLAGS)
--
2.23.0.700.g56cf767bdb-goog
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [kvm-unit-tests PATCH 4/4] Makefile: add "cxx-option" for C++ builds
2019-10-14 19:24 ` [kvm-unit-tests PATCH 0/4] Patches for clang compilation Bill Wendling
` (2 preceding siblings ...)
2019-10-14 19:24 ` [kvm-unit-tests PATCH 3/4] Makefile: use "-Werror" in cc-option Bill Wendling
@ 2019-10-14 19:24 ` Bill Wendling
2019-10-14 19:25 ` [kvm-unit-tests PATCH 0/4] Patches for clang compilation Bill Wendling
4 siblings, 0 replies; 37+ messages in thread
From: Bill Wendling @ 2019-10-14 19:24 UTC (permalink / raw)
To: kvm, pbonzini; +Cc: jmattson, sean.j.christopherson, Bill Wendling
The C++ compiler may not support all of the same flags as the C
compiler. Add a separate test for these flags.
Signed-off-by: Bill Wendling <morbo@google.com>
---
Makefile | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile
index 3ec0458..b9ae791 100644
--- a/Makefile
+++ b/Makefile
@@ -48,6 +48,8 @@ include $(SRCDIR)/$(TEST_DIR)/Makefile
cc-option = $(shell if $(CC) -Werror $(1) -S -o /dev/null -xc /dev/null \
> /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;)
+cxx-option = $(shell if $(CXX) -Werror $(1) -S -o /dev/null -xc++ /dev/null \
+ > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;)
COMMON_CFLAGS += -g $(autodepend-flags)
COMMON_CFLAGS += -Wall -Wwrite-strings -Wempty-body -Wuninitialized
@@ -68,13 +70,15 @@ COMMON_CFLAGS += $(fno_pic) $(no_pie)
COMMON_CFLAGS += $(call cc-option, -Wno-frame-address, "")
COMMON_CFLAGS += $(call cc-option, -Wclobbered, "")
COMMON_CFLAGS += $(call cc-option, -Wunused-but-set-parameter, "")
-COMMON_CFLAGS += $(call cc-option, -Wmissing-parameter-type, "")
-COMMON_CFLAGS += $(call cc-option, -Wold-style-declaration, "")
CFLAGS += $(COMMON_CFLAGS)
+CFLAGS += $(call cc-option, -Wmissing-parameter-type, "")
+CFLAGS += $(call cc-option, -Wold-style-declaration, "")
CFLAGS += -Woverride-init -Wmissing-prototypes -Wstrict-prototypes
CXXFLAGS += $(COMMON_CFLAGS)
+CXXFLAGS += $(call cxx-option, -Wmissing-parameter-type, "")
+CXXFLAGS += $(call cxx-option, -Wold-style-declaration, "")
autodepend-flags = -MMD -MF $(dir $*).$(notdir $*).d
--
2.23.0.700.g56cf767bdb-goog
^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [kvm-unit-tests PATCH 0/4] Patches for clang compilation
2019-10-14 19:24 ` [kvm-unit-tests PATCH 0/4] Patches for clang compilation Bill Wendling
` (3 preceding siblings ...)
2019-10-14 19:24 ` [kvm-unit-tests PATCH 4/4] Makefile: add "cxx-option" for C++ builds Bill Wendling
@ 2019-10-14 19:25 ` Bill Wendling
2019-10-14 23:56 ` Nadav Amit
4 siblings, 1 reply; 37+ messages in thread
From: Bill Wendling @ 2019-10-14 19:25 UTC (permalink / raw)
To: kvm list, Paolo Bonzini; +Cc: Jim Mattson, Sean Christopherson
Crap! I used what I thought were the correct command line arguments
for "git send-email", but it didn't add the "v2" bit. My apologies.
All of these patches should be v2 for the originals.
On Mon, Oct 14, 2019 at 12:24 PM Bill Wendling <morbo@google.com> wrote:
>
> These four patches fix compilation issues that clang's encountering.
>
> I ran the tests and there are no regressions with gcc.
>
> Bill Wendling (4):
> x86: emulator: use "SSE2" for the target
> pci: cast the masks to the appropriate size
> Makefile: use "-Werror" in cc-option
> Makefile: add "cxx-option" for C++ builds
>
> Makefile | 23 +++++++++++++++--------
> lib/pci.c | 3 ++-
> x86/emulator.c | 2 +-
> 3 files changed, 18 insertions(+), 10 deletions(-)
>
> --
> 2.23.0.700.g56cf767bdb-goog
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [kvm-unit-tests PATCH 0/4] Patches for clang compilation
2019-10-14 19:25 ` [kvm-unit-tests PATCH 0/4] Patches for clang compilation Bill Wendling
@ 2019-10-14 23:56 ` Nadav Amit
2019-10-15 0:04 ` Bill Wendling
0 siblings, 1 reply; 37+ messages in thread
From: Nadav Amit @ 2019-10-14 23:56 UTC (permalink / raw)
To: Bill Wendling; +Cc: kvm list, Paolo Bonzini, Jim Mattson, Sean Christopherson
On Oct 14, 2019, at 12:25 PM, Bill Wendling <morbo@google.com> wrote:
>
> Crap! I used what I thought were the correct command line arguments
> for "git send-email", but it didn't add the "v2" bit. My apologies.
> All of these patches should be v2 for the originals.
I recommend that you send them again with v2 in the title to avoid
confusion.
^ permalink raw reply [flat|nested] 37+ messages in thread
* [kvm-unit-tests PATCH v2 0/4] Patches for clang compilation
2019-10-10 18:35 [kvm-unit-tests PATCH 0/3] Patches for clang compilation Bill Wendling
` (3 preceding siblings ...)
2019-10-14 19:24 ` [kvm-unit-tests PATCH 0/4] Patches for clang compilation Bill Wendling
@ 2019-10-15 0:04 ` Bill Wendling
2019-10-15 0:04 ` [kvm-unit-tests PATCH v2 1/4] x86: emulator: use "SSE2" for the target Bill Wendling
` (4 more replies)
4 siblings, 5 replies; 37+ messages in thread
From: Bill Wendling @ 2019-10-15 0:04 UTC (permalink / raw)
To: kvm, pbonzini; +Cc: jmattson, sean.j.christopherson, Bill Wendling
These four patches fix compilation issues that clang's encountering.
I ran the tests and there are no regressions with gcc.
Bill Wendling (4):
x86: emulator: use "SSE2" for the target
pci: cast the masks to the appropriate size
Makefile: use "-Werror" in cc-option
Makefile: add "cxx-option" for C++ builds
Makefile | 23 +++++++++++++++--------
lib/pci.c | 3 ++-
x86/emulator.c | 2 +-
3 files changed, 18 insertions(+), 10 deletions(-)
--
2.23.0.700.g56cf767bdb-goog
^ permalink raw reply [flat|nested] 37+ messages in thread
* [kvm-unit-tests PATCH v2 1/4] x86: emulator: use "SSE2" for the target
2019-10-15 0:04 ` [kvm-unit-tests PATCH v2 " Bill Wendling
@ 2019-10-15 0:04 ` Bill Wendling
2019-10-15 0:04 ` [kvm-unit-tests PATCH v2 2/4] pci: cast the masks to the appropriate size Bill Wendling
` (3 subsequent siblings)
4 siblings, 0 replies; 37+ messages in thread
From: Bill Wendling @ 2019-10-15 0:04 UTC (permalink / raw)
To: kvm, pbonzini; +Cc: jmattson, sean.j.christopherson, Bill Wendling
The movdqu and movapd instructions are SSE2 instructions. Clang
interprets the __attribute__((target("sse"))) as allowing SSE only
instructions. Using SSE2 instructions cause an error.
Signed-off-by: Bill Wendling <morbo@google.com>
---
x86/emulator.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/x86/emulator.c b/x86/emulator.c
index 621caf9..bec0154 100644
--- a/x86/emulator.c
+++ b/x86/emulator.c
@@ -657,7 +657,7 @@ static bool sseeq(sse_union *v1, sse_union *v2)
return ok;
}
-static __attribute__((target("sse"))) void test_sse(sse_union *mem)
+static __attribute__((target("sse2"))) void test_sse(sse_union *mem)
{
sse_union v;
--
2.23.0.700.g56cf767bdb-goog
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [kvm-unit-tests PATCH v2 2/4] pci: cast the masks to the appropriate size
2019-10-15 0:04 ` [kvm-unit-tests PATCH v2 " Bill Wendling
2019-10-15 0:04 ` [kvm-unit-tests PATCH v2 1/4] x86: emulator: use "SSE2" for the target Bill Wendling
@ 2019-10-15 0:04 ` Bill Wendling
2019-10-15 0:04 ` [kvm-unit-tests PATCH v2 3/4] Makefile: use "-Werror" in cc-option Bill Wendling
` (2 subsequent siblings)
4 siblings, 0 replies; 37+ messages in thread
From: Bill Wendling @ 2019-10-15 0:04 UTC (permalink / raw)
To: kvm, pbonzini; +Cc: jmattson, sean.j.christopherson, Bill Wendling
At this point, we're dealing with 32-bit addresses, therefore downcast
the masks to 32-bits.
Signed-off-by: Bill Wendling <morbo@google.com>
---
lib/pci.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/pci.c b/lib/pci.c
index daa33e1..1b85411 100644
--- a/lib/pci.c
+++ b/lib/pci.c
@@ -107,7 +107,8 @@ pcidevaddr_t pci_find_dev(uint16_t vendor_id, uint16_t device_id)
uint32_t pci_bar_mask(uint32_t bar)
{
return (bar & PCI_BASE_ADDRESS_SPACE_IO) ?
- PCI_BASE_ADDRESS_IO_MASK : PCI_BASE_ADDRESS_MEM_MASK;
+ (uint32_t)PCI_BASE_ADDRESS_IO_MASK :
+ (uint32_t)PCI_BASE_ADDRESS_MEM_MASK;
}
uint32_t pci_bar_get(struct pci_dev *dev, int bar_num)
--
2.23.0.700.g56cf767bdb-goog
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [kvm-unit-tests PATCH v2 3/4] Makefile: use "-Werror" in cc-option
2019-10-15 0:04 ` [kvm-unit-tests PATCH v2 " Bill Wendling
2019-10-15 0:04 ` [kvm-unit-tests PATCH v2 1/4] x86: emulator: use "SSE2" for the target Bill Wendling
2019-10-15 0:04 ` [kvm-unit-tests PATCH v2 2/4] pci: cast the masks to the appropriate size Bill Wendling
@ 2019-10-15 0:04 ` Bill Wendling
2019-10-15 0:04 ` [kvm-unit-tests PATCH v2 4/4] Makefile: add "cxx-option" for C++ builds Bill Wendling
2019-10-30 21:04 ` [kvm-unit-tests PATCH v3 0/6] Patches for clang compilation Bill Wendling
4 siblings, 0 replies; 37+ messages in thread
From: Bill Wendling @ 2019-10-15 0:04 UTC (permalink / raw)
To: kvm, pbonzini; +Cc: jmattson, sean.j.christopherson, Bill Wendling
The "cc-option" macro should use "-Werror" to determine if a flag is
supported. Otherwise the test may not return a nonzero result. Also
conditionalize some of the warning flags which aren't supported by
clang.
Signed-off-by: Bill Wendling <morbo@google.com>
---
Makefile | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/Makefile b/Makefile
index 32414dc..3ec0458 100644
--- a/Makefile
+++ b/Makefile
@@ -46,30 +46,33 @@ include $(SRCDIR)/$(TEST_DIR)/Makefile
# cc-option
# Usage: OP_CFLAGS+=$(call cc-option, -falign-functions=0, -malign-functions=0)
-cc-option = $(shell if $(CC) $(1) -S -o /dev/null -xc /dev/null \
+cc-option = $(shell if $(CC) -Werror $(1) -S -o /dev/null -xc /dev/null \
> /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;)
COMMON_CFLAGS += -g $(autodepend-flags)
-COMMON_CFLAGS += -Wall -Wwrite-strings -Wclobbered -Wempty-body -Wuninitialized
-COMMON_CFLAGS += -Wignored-qualifiers -Wunused-but-set-parameter
-COMMON_CFLAGS += -Werror
+COMMON_CFLAGS += -Wall -Wwrite-strings -Wempty-body -Wuninitialized
+COMMON_CFLAGS += -Wignored-qualifiers -Werror
+
frame-pointer-flag=-f$(if $(KEEP_FRAME_POINTER),no-,)omit-frame-pointer
fomit_frame_pointer := $(call cc-option, $(frame-pointer-flag), "")
fnostack_protector := $(call cc-option, -fno-stack-protector, "")
fnostack_protector_all := $(call cc-option, -fno-stack-protector-all, "")
-wno_frame_address := $(call cc-option, -Wno-frame-address, "")
fno_pic := $(call cc-option, -fno-pic, "")
no_pie := $(call cc-option, -no-pie, "")
COMMON_CFLAGS += $(fomit_frame_pointer)
COMMON_CFLAGS += $(fno_stack_protector)
COMMON_CFLAGS += $(fno_stack_protector_all)
-COMMON_CFLAGS += $(wno_frame_address)
COMMON_CFLAGS += $(if $(U32_LONG_FMT),-D__U32_LONG_FMT__,)
COMMON_CFLAGS += $(fno_pic) $(no_pie)
+COMMON_CFLAGS += $(call cc-option, -Wno-frame-address, "")
+COMMON_CFLAGS += $(call cc-option, -Wclobbered, "")
+COMMON_CFLAGS += $(call cc-option, -Wunused-but-set-parameter, "")
+COMMON_CFLAGS += $(call cc-option, -Wmissing-parameter-type, "")
+COMMON_CFLAGS += $(call cc-option, -Wold-style-declaration, "")
+
CFLAGS += $(COMMON_CFLAGS)
-CFLAGS += -Wmissing-parameter-type -Wold-style-declaration -Woverride-init
-CFLAGS += -Wmissing-prototypes -Wstrict-prototypes
+CFLAGS += -Woverride-init -Wmissing-prototypes -Wstrict-prototypes
CXXFLAGS += $(COMMON_CFLAGS)
--
2.23.0.700.g56cf767bdb-goog
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [kvm-unit-tests PATCH v2 4/4] Makefile: add "cxx-option" for C++ builds
2019-10-15 0:04 ` [kvm-unit-tests PATCH v2 " Bill Wendling
` (2 preceding siblings ...)
2019-10-15 0:04 ` [kvm-unit-tests PATCH v2 3/4] Makefile: use "-Werror" in cc-option Bill Wendling
@ 2019-10-15 0:04 ` Bill Wendling
2019-10-30 21:04 ` [kvm-unit-tests PATCH v3 0/6] Patches for clang compilation Bill Wendling
4 siblings, 0 replies; 37+ messages in thread
From: Bill Wendling @ 2019-10-15 0:04 UTC (permalink / raw)
To: kvm, pbonzini; +Cc: jmattson, sean.j.christopherson, Bill Wendling
The C++ compiler may not support all of the same flags as the C
compiler. Add a separate test for these flags.
Signed-off-by: Bill Wendling <morbo@google.com>
---
Makefile | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile
index 3ec0458..b9ae791 100644
--- a/Makefile
+++ b/Makefile
@@ -48,6 +48,8 @@ include $(SRCDIR)/$(TEST_DIR)/Makefile
cc-option = $(shell if $(CC) -Werror $(1) -S -o /dev/null -xc /dev/null \
> /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;)
+cxx-option = $(shell if $(CXX) -Werror $(1) -S -o /dev/null -xc++ /dev/null \
+ > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;)
COMMON_CFLAGS += -g $(autodepend-flags)
COMMON_CFLAGS += -Wall -Wwrite-strings -Wempty-body -Wuninitialized
@@ -68,13 +70,15 @@ COMMON_CFLAGS += $(fno_pic) $(no_pie)
COMMON_CFLAGS += $(call cc-option, -Wno-frame-address, "")
COMMON_CFLAGS += $(call cc-option, -Wclobbered, "")
COMMON_CFLAGS += $(call cc-option, -Wunused-but-set-parameter, "")
-COMMON_CFLAGS += $(call cc-option, -Wmissing-parameter-type, "")
-COMMON_CFLAGS += $(call cc-option, -Wold-style-declaration, "")
CFLAGS += $(COMMON_CFLAGS)
+CFLAGS += $(call cc-option, -Wmissing-parameter-type, "")
+CFLAGS += $(call cc-option, -Wold-style-declaration, "")
CFLAGS += -Woverride-init -Wmissing-prototypes -Wstrict-prototypes
CXXFLAGS += $(COMMON_CFLAGS)
+CXXFLAGS += $(call cxx-option, -Wmissing-parameter-type, "")
+CXXFLAGS += $(call cxx-option, -Wold-style-declaration, "")
autodepend-flags = -MMD -MF $(dir $*).$(notdir $*).d
--
2.23.0.700.g56cf767bdb-goog
^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [kvm-unit-tests PATCH 0/4] Patches for clang compilation
2019-10-14 23:56 ` Nadav Amit
@ 2019-10-15 0:04 ` Bill Wendling
0 siblings, 0 replies; 37+ messages in thread
From: Bill Wendling @ 2019-10-15 0:04 UTC (permalink / raw)
To: Nadav Amit; +Cc: kvm list, Paolo Bonzini, Jim Mattson, Sean Christopherson
Done.
On Mon, Oct 14, 2019 at 4:56 PM Nadav Amit <nadav.amit@gmail.com> wrote:
>
> On Oct 14, 2019, at 12:25 PM, Bill Wendling <morbo@google.com> wrote:
> >
> > Crap! I used what I thought were the correct command line arguments
> > for "git send-email", but it didn't add the "v2" bit. My apologies.
> > All of these patches should be v2 for the originals.
>
> I recommend that you send them again with v2 in the title to avoid
> confusion.
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [kvm-unit-tests PATCH 3/3] Makefile: use "-Werror" in cc-option
2019-10-10 18:35 ` [kvm-unit-tests PATCH 3/3] Makefile: use "-Werror" in cc-option Bill Wendling
@ 2019-10-15 7:29 ` Thomas Huth
2019-10-15 7:57 ` Bill Wendling
0 siblings, 1 reply; 37+ messages in thread
From: Thomas Huth @ 2019-10-15 7:29 UTC (permalink / raw)
To: Bill Wendling, kvm, pbonzini; +Cc: jmattson
On 10/10/2019 20.35, Bill Wendling wrote:
> The "cc-option" macro should use "-Werror" to determine if a flag is
> supported. Otherwise the test may not return a nonzero result. Also
> conditionalize some of the warning flags which aren't supported by
> clang.
>
> Signed-off-by: Bill Wendling <morbo@google.com>
> ---
> Makefile | 19 +++++++++++--------
> 1 file changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 32414dc..3ec0458 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -46,30 +46,33 @@ include $(SRCDIR)/$(TEST_DIR)/Makefile
> # cc-option
> # Usage: OP_CFLAGS+=$(call cc-option, -falign-functions=0, -malign-functions=0)
>
> -cc-option = $(shell if $(CC) $(1) -S -o /dev/null -xc /dev/null \
> +cc-option = $(shell if $(CC) -Werror $(1) -S -o /dev/null -xc /dev/null \
> > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;)
>
> COMMON_CFLAGS += -g $(autodepend-flags)
> -COMMON_CFLAGS += -Wall -Wwrite-strings -Wclobbered -Wempty-body -Wuninitialized
> -COMMON_CFLAGS += -Wignored-qualifiers -Wunused-but-set-parameter
> -COMMON_CFLAGS += -Werror
> +COMMON_CFLAGS += -Wall -Wwrite-strings -Wempty-body -Wuninitialized
> +COMMON_CFLAGS += -Wignored-qualifiers -Werror
> +
> frame-pointer-flag=-f$(if $(KEEP_FRAME_POINTER),no-,)omit-frame-pointer
> fomit_frame_pointer := $(call cc-option, $(frame-pointer-flag), "")
> fnostack_protector := $(call cc-option, -fno-stack-protector, "")
> fnostack_protector_all := $(call cc-option, -fno-stack-protector-all, "")
> -wno_frame_address := $(call cc-option, -Wno-frame-address, "")
> fno_pic := $(call cc-option, -fno-pic, "")
> no_pie := $(call cc-option, -no-pie, "")
> COMMON_CFLAGS += $(fomit_frame_pointer)
> COMMON_CFLAGS += $(fno_stack_protector)
> COMMON_CFLAGS += $(fno_stack_protector_all)
> -COMMON_CFLAGS += $(wno_frame_address)
> COMMON_CFLAGS += $(if $(U32_LONG_FMT),-D__U32_LONG_FMT__,)
> COMMON_CFLAGS += $(fno_pic) $(no_pie)
>
> +COMMON_CFLAGS += $(call cc-option, -Wno-frame-address, "")
I think the old code used ":=" on purpose, so that the test is only done
once. With your new code, the test is now done each time COMMON_CFLAGS
gets evaluated, i.e. for each compiled object.
Could you please rewrite this test to use the ":=" detour for all lines
that call cc-option?
Thanks,
Thomas
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [kvm-unit-tests PATCH 3/3] Makefile: use "-Werror" in cc-option
2019-10-15 7:29 ` Thomas Huth
@ 2019-10-15 7:57 ` Bill Wendling
2019-10-15 8:12 ` Thomas Huth
0 siblings, 1 reply; 37+ messages in thread
From: Bill Wendling @ 2019-10-15 7:57 UTC (permalink / raw)
To: Thomas Huth; +Cc: kvm list, Paolo Bonzini, Jim Mattson
On Tue, Oct 15, 2019 at 12:29 AM Thomas Huth <thuth@redhat.com> wrote:
>
> On 10/10/2019 20.35, Bill Wendling wrote:
> > The "cc-option" macro should use "-Werror" to determine if a flag is
> > supported. Otherwise the test may not return a nonzero result. Also
> > conditionalize some of the warning flags which aren't supported by
> > clang.
> >
> > Signed-off-by: Bill Wendling <morbo@google.com>
> > ---
> > Makefile | 19 +++++++++++--------
> > 1 file changed, 11 insertions(+), 8 deletions(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 32414dc..3ec0458 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -46,30 +46,33 @@ include $(SRCDIR)/$(TEST_DIR)/Makefile
> > # cc-option
> > # Usage: OP_CFLAGS+=$(call cc-option, -falign-functions=0, -malign-functions=0)
> >
> > -cc-option = $(shell if $(CC) $(1) -S -o /dev/null -xc /dev/null \
> > +cc-option = $(shell if $(CC) -Werror $(1) -S -o /dev/null -xc /dev/null \
> > > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;)
> >
> > COMMON_CFLAGS += -g $(autodepend-flags)
> > -COMMON_CFLAGS += -Wall -Wwrite-strings -Wclobbered -Wempty-body -Wuninitialized
> > -COMMON_CFLAGS += -Wignored-qualifiers -Wunused-but-set-parameter
> > -COMMON_CFLAGS += -Werror
> > +COMMON_CFLAGS += -Wall -Wwrite-strings -Wempty-body -Wuninitialized
> > +COMMON_CFLAGS += -Wignored-qualifiers -Werror
> > +
> > frame-pointer-flag=-f$(if $(KEEP_FRAME_POINTER),no-,)omit-frame-pointer
> > fomit_frame_pointer := $(call cc-option, $(frame-pointer-flag), "")
> > fnostack_protector := $(call cc-option, -fno-stack-protector, "")
> > fnostack_protector_all := $(call cc-option, -fno-stack-protector-all, "")
> > -wno_frame_address := $(call cc-option, -Wno-frame-address, "")
> > fno_pic := $(call cc-option, -fno-pic, "")
> > no_pie := $(call cc-option, -no-pie, "")
> > COMMON_CFLAGS += $(fomit_frame_pointer)
> > COMMON_CFLAGS += $(fno_stack_protector)
> > COMMON_CFLAGS += $(fno_stack_protector_all)
> > -COMMON_CFLAGS += $(wno_frame_address)
> > COMMON_CFLAGS += $(if $(U32_LONG_FMT),-D__U32_LONG_FMT__,)
> > COMMON_CFLAGS += $(fno_pic) $(no_pie)
> >
> > +COMMON_CFLAGS += $(call cc-option, -Wno-frame-address, "")
>
> I think the old code used ":=" on purpose, so that the test is only done
> once. With your new code, the test is now done each time COMMON_CFLAGS
> gets evaluated, i.e. for each compiled object.
>
> Could you please rewrite this test to use the ":=" detour for all lines
> that call cc-option?
>
Does it rerun the "cc-option" call if the COMMON_CFLAGS is initially
defined with ":="? I ask because the kernel calls its version of
cc-option in the way I have it above, but the original definition of
the variable uses ":=".
-bw
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [kvm-unit-tests PATCH 3/3] Makefile: use "-Werror" in cc-option
2019-10-15 7:57 ` Bill Wendling
@ 2019-10-15 8:12 ` Thomas Huth
0 siblings, 0 replies; 37+ messages in thread
From: Thomas Huth @ 2019-10-15 8:12 UTC (permalink / raw)
To: Bill Wendling; +Cc: kvm list, Paolo Bonzini, Jim Mattson
On 15/10/2019 09.57, Bill Wendling wrote:
> On Tue, Oct 15, 2019 at 12:29 AM Thomas Huth <thuth@redhat.com> wrote:
>>
>> On 10/10/2019 20.35, Bill Wendling wrote:
>>> The "cc-option" macro should use "-Werror" to determine if a flag is
>>> supported. Otherwise the test may not return a nonzero result. Also
>>> conditionalize some of the warning flags which aren't supported by
>>> clang.
>>>
>>> Signed-off-by: Bill Wendling <morbo@google.com>
>>> ---
>>> Makefile | 19 +++++++++++--------
>>> 1 file changed, 11 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/Makefile b/Makefile
>>> index 32414dc..3ec0458 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -46,30 +46,33 @@ include $(SRCDIR)/$(TEST_DIR)/Makefile
>>> # cc-option
>>> # Usage: OP_CFLAGS+=$(call cc-option, -falign-functions=0, -malign-functions=0)
>>>
>>> -cc-option = $(shell if $(CC) $(1) -S -o /dev/null -xc /dev/null \
>>> +cc-option = $(shell if $(CC) -Werror $(1) -S -o /dev/null -xc /dev/null \
>>> > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;)
>>>
>>> COMMON_CFLAGS += -g $(autodepend-flags)
>>> -COMMON_CFLAGS += -Wall -Wwrite-strings -Wclobbered -Wempty-body -Wuninitialized
>>> -COMMON_CFLAGS += -Wignored-qualifiers -Wunused-but-set-parameter
>>> -COMMON_CFLAGS += -Werror
>>> +COMMON_CFLAGS += -Wall -Wwrite-strings -Wempty-body -Wuninitialized
>>> +COMMON_CFLAGS += -Wignored-qualifiers -Werror
>>> +
>>> frame-pointer-flag=-f$(if $(KEEP_FRAME_POINTER),no-,)omit-frame-pointer
>>> fomit_frame_pointer := $(call cc-option, $(frame-pointer-flag), "")
>>> fnostack_protector := $(call cc-option, -fno-stack-protector, "")
>>> fnostack_protector_all := $(call cc-option, -fno-stack-protector-all, "")
>>> -wno_frame_address := $(call cc-option, -Wno-frame-address, "")
>>> fno_pic := $(call cc-option, -fno-pic, "")
>>> no_pie := $(call cc-option, -no-pie, "")
>>> COMMON_CFLAGS += $(fomit_frame_pointer)
>>> COMMON_CFLAGS += $(fno_stack_protector)
>>> COMMON_CFLAGS += $(fno_stack_protector_all)
>>> -COMMON_CFLAGS += $(wno_frame_address)
>>> COMMON_CFLAGS += $(if $(U32_LONG_FMT),-D__U32_LONG_FMT__,)
>>> COMMON_CFLAGS += $(fno_pic) $(no_pie)
>>>
>>> +COMMON_CFLAGS += $(call cc-option, -Wno-frame-address, "")
>>
>> I think the old code used ":=" on purpose, so that the test is only done
>> once. With your new code, the test is now done each time COMMON_CFLAGS
>> gets evaluated, i.e. for each compiled object.
>>
>> Could you please rewrite this test to use the ":=" detour for all lines
>> that call cc-option?
>>
> Does it rerun the "cc-option" call if the COMMON_CFLAGS is initially
> defined with ":="?
It does not rerun the "cc-option" call in that case, but compilation
fails. I think it's likely due to the line that contains the "$(if
$(U32_LONG_FMT),-D__U32_LONG_FMT__,)" since that likely needs to be
evaluated dynamically.
Thomas
^ permalink raw reply [flat|nested] 37+ messages in thread
* [kvm-unit-tests PATCH v3 0/6] Patches for clang compilation
2019-10-15 0:04 ` [kvm-unit-tests PATCH v2 " Bill Wendling
` (3 preceding siblings ...)
2019-10-15 0:04 ` [kvm-unit-tests PATCH v2 4/4] Makefile: add "cxx-option" for C++ builds Bill Wendling
@ 2019-10-30 21:04 ` Bill Wendling
2019-10-30 21:04 ` [kvm-unit-tests PATCH v3 1/6] x86: emulator: use "SSE2" for the target Bill Wendling
` (5 more replies)
4 siblings, 6 replies; 37+ messages in thread
From: Bill Wendling @ 2019-10-30 21:04 UTC (permalink / raw)
To: kvm, pbonzini, thuth; +Cc: jmattson, sean.j.christopherson, Bill Wendling
This series of patches includes extra from the original patch set:
- Changes to the "use -Werror in cc-option" patch to address Thomas
Huth's comments.
- Two fixes for code clang warned about: shifting negative numbers, and
misuse of the "registers for local variables" feature.
Bill Wendling (6):
x86: emulator: use "SSE2" for the target
pci: cast the masks to the appropriate size
Makefile: use "-Werror" in cc-option
Makefile: add "cxx-option" for C++ builds
x86: use a non-negative number in shift
x86: use inline asm to retrieve stack pointer
Makefile | 30 +++++++++++++++++++++++-------
lib/pci.c | 3 ++-
x86/emulator.c | 2 +-
x86/svm.c | 2 +-
x86/vmx_tests.c | 8 ++++++--
5 files changed, 33 insertions(+), 12 deletions(-)
--
2.24.0.rc1.363.gb1bccd3e3d-goog
^ permalink raw reply [flat|nested] 37+ messages in thread
* [kvm-unit-tests PATCH v3 1/6] x86: emulator: use "SSE2" for the target
2019-10-30 21:04 ` [kvm-unit-tests PATCH v3 0/6] Patches for clang compilation Bill Wendling
@ 2019-10-30 21:04 ` Bill Wendling
2019-11-28 7:31 ` Thomas Huth
2019-10-30 21:04 ` [kvm-unit-tests PATCH v3 2/6] pci: cast the masks to the appropriate size Bill Wendling
` (4 subsequent siblings)
5 siblings, 1 reply; 37+ messages in thread
From: Bill Wendling @ 2019-10-30 21:04 UTC (permalink / raw)
To: kvm, pbonzini, thuth; +Cc: jmattson, sean.j.christopherson, Bill Wendling
The movdqu and movapd instructions are SSE2 instructions. Clang
interprets the __attribute__((target("sse"))) as allowing SSE only
instructions. Using SSE2 instructions cause an error.
Signed-off-by: Bill Wendling <morbo@google.com>
---
x86/emulator.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/x86/emulator.c b/x86/emulator.c
index 621caf9..bec0154 100644
--- a/x86/emulator.c
+++ b/x86/emulator.c
@@ -657,7 +657,7 @@ static bool sseeq(sse_union *v1, sse_union *v2)
return ok;
}
-static __attribute__((target("sse"))) void test_sse(sse_union *mem)
+static __attribute__((target("sse2"))) void test_sse(sse_union *mem)
{
sse_union v;
--
2.24.0.rc1.363.gb1bccd3e3d-goog
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [kvm-unit-tests PATCH v3 2/6] pci: cast the masks to the appropriate size
2019-10-30 21:04 ` [kvm-unit-tests PATCH v3 0/6] Patches for clang compilation Bill Wendling
2019-10-30 21:04 ` [kvm-unit-tests PATCH v3 1/6] x86: emulator: use "SSE2" for the target Bill Wendling
@ 2019-10-30 21:04 ` Bill Wendling
2019-11-08 8:39 ` Thomas Huth
2019-10-30 21:04 ` [kvm-unit-tests PATCH v3 3/6] Makefile: use "-Werror" in cc-option Bill Wendling
` (3 subsequent siblings)
5 siblings, 1 reply; 37+ messages in thread
From: Bill Wendling @ 2019-10-30 21:04 UTC (permalink / raw)
To: kvm, pbonzini, thuth; +Cc: jmattson, sean.j.christopherson, Bill Wendling
At this point, we're dealing with 32-bit addresses, therefore downcast
the masks to 32-bits.
Signed-off-by: Bill Wendling <morbo@google.com>
---
lib/pci.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/pci.c b/lib/pci.c
index daa33e1..1b85411 100644
--- a/lib/pci.c
+++ b/lib/pci.c
@@ -107,7 +107,8 @@ pcidevaddr_t pci_find_dev(uint16_t vendor_id, uint16_t device_id)
uint32_t pci_bar_mask(uint32_t bar)
{
return (bar & PCI_BASE_ADDRESS_SPACE_IO) ?
- PCI_BASE_ADDRESS_IO_MASK : PCI_BASE_ADDRESS_MEM_MASK;
+ (uint32_t)PCI_BASE_ADDRESS_IO_MASK :
+ (uint32_t)PCI_BASE_ADDRESS_MEM_MASK;
}
uint32_t pci_bar_get(struct pci_dev *dev, int bar_num)
--
2.24.0.rc1.363.gb1bccd3e3d-goog
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [kvm-unit-tests PATCH v3 3/6] Makefile: use "-Werror" in cc-option
2019-10-30 21:04 ` [kvm-unit-tests PATCH v3 0/6] Patches for clang compilation Bill Wendling
2019-10-30 21:04 ` [kvm-unit-tests PATCH v3 1/6] x86: emulator: use "SSE2" for the target Bill Wendling
2019-10-30 21:04 ` [kvm-unit-tests PATCH v3 2/6] pci: cast the masks to the appropriate size Bill Wendling
@ 2019-10-30 21:04 ` Bill Wendling
2019-11-08 8:43 ` Thomas Huth
2019-10-30 21:04 ` [kvm-unit-tests PATCH v3 4/6] Makefile: add "cxx-option" for C++ builds Bill Wendling
` (2 subsequent siblings)
5 siblings, 1 reply; 37+ messages in thread
From: Bill Wendling @ 2019-10-30 21:04 UTC (permalink / raw)
To: kvm, pbonzini, thuth; +Cc: jmattson, sean.j.christopherson, Bill Wendling
The "cc-option" macro should use "-Werror" to determine if a flag is
supported. Otherwise the test may not return a nonzero result. Also
conditionalize some of the warning flags which aren't supported by
clang.
Signed-off-by: Bill Wendling <morbo@google.com>
---
Makefile | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/Makefile b/Makefile
index 32414dc..6201c45 100644
--- a/Makefile
+++ b/Makefile
@@ -46,13 +46,13 @@ include $(SRCDIR)/$(TEST_DIR)/Makefile
# cc-option
# Usage: OP_CFLAGS+=$(call cc-option, -falign-functions=0, -malign-functions=0)
-cc-option = $(shell if $(CC) $(1) -S -o /dev/null -xc /dev/null \
+cc-option = $(shell if $(CC) -Werror $(1) -S -o /dev/null -xc /dev/null \
> /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;)
COMMON_CFLAGS += -g $(autodepend-flags)
-COMMON_CFLAGS += -Wall -Wwrite-strings -Wclobbered -Wempty-body -Wuninitialized
-COMMON_CFLAGS += -Wignored-qualifiers -Wunused-but-set-parameter
-COMMON_CFLAGS += -Werror
+COMMON_CFLAGS += -Wall -Wwrite-strings -Wempty-body -Wuninitialized
+COMMON_CFLAGS += -Wignored-qualifiers -Werror
+
frame-pointer-flag=-f$(if $(KEEP_FRAME_POINTER),no-,)omit-frame-pointer
fomit_frame_pointer := $(call cc-option, $(frame-pointer-flag), "")
fnostack_protector := $(call cc-option, -fno-stack-protector, "")
@@ -60,16 +60,20 @@ fnostack_protector_all := $(call cc-option, -fno-stack-protector-all, "")
wno_frame_address := $(call cc-option, -Wno-frame-address, "")
fno_pic := $(call cc-option, -fno-pic, "")
no_pie := $(call cc-option, -no-pie, "")
+wclobbered := $(call cc-option, -Wclobbered, "")
+wunused_but_set_parameter := $(call cc-option, -Wunused-but-set-parameter, "")
+
COMMON_CFLAGS += $(fomit_frame_pointer)
COMMON_CFLAGS += $(fno_stack_protector)
COMMON_CFLAGS += $(fno_stack_protector_all)
COMMON_CFLAGS += $(wno_frame_address)
COMMON_CFLAGS += $(if $(U32_LONG_FMT),-D__U32_LONG_FMT__,)
COMMON_CFLAGS += $(fno_pic) $(no_pie)
+COMMON_CFLAGS += $(wclobbered)
+COMMON_CFLAGS += $(wunused_but_set_parameter)
CFLAGS += $(COMMON_CFLAGS)
-CFLAGS += -Wmissing-parameter-type -Wold-style-declaration -Woverride-init
-CFLAGS += -Wmissing-prototypes -Wstrict-prototypes
+CFLAGS += -Woverride-init -Wmissing-prototypes -Wstrict-prototypes
CXXFLAGS += $(COMMON_CFLAGS)
--
2.24.0.rc1.363.gb1bccd3e3d-goog
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [kvm-unit-tests PATCH v3 4/6] Makefile: add "cxx-option" for C++ builds
2019-10-30 21:04 ` [kvm-unit-tests PATCH v3 0/6] Patches for clang compilation Bill Wendling
` (2 preceding siblings ...)
2019-10-30 21:04 ` [kvm-unit-tests PATCH v3 3/6] Makefile: use "-Werror" in cc-option Bill Wendling
@ 2019-10-30 21:04 ` Bill Wendling
2019-11-08 8:48 ` Thomas Huth
2019-10-30 21:04 ` [kvm-unit-tests PATCH v3 5/6] x86: use a non-negative number in shift Bill Wendling
2019-10-30 21:04 ` [kvm-unit-tests PATCH v3 6/6] x86: use inline asm to retrieve stack pointer Bill Wendling
5 siblings, 1 reply; 37+ messages in thread
From: Bill Wendling @ 2019-10-30 21:04 UTC (permalink / raw)
To: kvm, pbonzini, thuth; +Cc: jmattson, sean.j.christopherson, Bill Wendling
The C++ compiler may not support all of the same flags as the C
compiler. Add a separate test for these flags.
Signed-off-by: Bill Wendling <morbo@google.com>
---
Makefile | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index 6201c45..9cb47e6 100644
--- a/Makefile
+++ b/Makefile
@@ -48,6 +48,8 @@ include $(SRCDIR)/$(TEST_DIR)/Makefile
cc-option = $(shell if $(CC) -Werror $(1) -S -o /dev/null -xc /dev/null \
> /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;)
+cxx-option = $(shell if $(CXX) -Werror $(1) -S -o /dev/null -xc++ /dev/null \
+ > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;)
COMMON_CFLAGS += -g $(autodepend-flags)
COMMON_CFLAGS += -Wall -Wwrite-strings -Wempty-body -Wuninitialized
@@ -73,9 +75,19 @@ COMMON_CFLAGS += $(wclobbered)
COMMON_CFLAGS += $(wunused_but_set_parameter)
CFLAGS += $(COMMON_CFLAGS)
+CXXFLAGS += $(COMMON_CFLAGS)
+
+wmissing_parameter_type := $(call cc-option, -Wmissing-parameter-type, "")
+wold_style_declaration := $(call cc-option, -Wold-style-declaration, "")
+CFLAGS += $(wmissing_parameter_type)
+CFLAGS += $(wold_style_declaration)
CFLAGS += -Woverride-init -Wmissing-prototypes -Wstrict-prototypes
-CXXFLAGS += $(COMMON_CFLAGS)
+# Clang's C++ compiler doesn't support some of the flags its C compiler does.
+wmissing_parameter_type := $(call cxx-option, -Wmissing-parameter-type, "")
+wold_style_declaration := $(call cxx-option, -Wold-style-declaration, "")
+CXXFLAGS += $(wmissing_parameter_type)
+CXXFLAGS += $(wold_style_declaration)
autodepend-flags = -MMD -MF $(dir $*).$(notdir $*).d
--
2.24.0.rc1.363.gb1bccd3e3d-goog
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [kvm-unit-tests PATCH v3 5/6] x86: use a non-negative number in shift
2019-10-30 21:04 ` [kvm-unit-tests PATCH v3 0/6] Patches for clang compilation Bill Wendling
` (3 preceding siblings ...)
2019-10-30 21:04 ` [kvm-unit-tests PATCH v3 4/6] Makefile: add "cxx-option" for C++ builds Bill Wendling
@ 2019-10-30 21:04 ` Bill Wendling
2019-11-08 8:31 ` Thomas Huth
2019-10-30 21:04 ` [kvm-unit-tests PATCH v3 6/6] x86: use inline asm to retrieve stack pointer Bill Wendling
5 siblings, 1 reply; 37+ messages in thread
From: Bill Wendling @ 2019-10-30 21:04 UTC (permalink / raw)
To: kvm, pbonzini, thuth; +Cc: jmattson, sean.j.christopherson, Bill Wendling
Shifting a negative number is undefined. Clang complains about it:
x86/svm.c:1131:38: error: shifting a negative signed value is undefined [-Werror,-Wshift-negative-value]
test->vmcb->control.tsc_offset = TSC_OFFSET_VALUE;
Using "~0ull" results in identical asm code:
before: movabsq $-281474976710656, %rsi
after: movabsq $-281474976710656, %rsi
Signed-off-by: Bill Wendling <morbo@google.com>
---
x86/svm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/x86/svm.c b/x86/svm.c
index 4ddfaa4..cef43d5 100644
--- a/x86/svm.c
+++ b/x86/svm.c
@@ -1122,7 +1122,7 @@ static bool npt_rw_l1mmio_check(struct test *test)
}
#define TSC_ADJUST_VALUE (1ll << 32)
-#define TSC_OFFSET_VALUE (-1ll << 48)
+#define TSC_OFFSET_VALUE (~0ull << 48)
static bool ok;
static void tsc_adjust_prepare(struct test *test)
--
2.24.0.rc1.363.gb1bccd3e3d-goog
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [kvm-unit-tests PATCH v3 6/6] x86: use inline asm to retrieve stack pointer
2019-10-30 21:04 ` [kvm-unit-tests PATCH v3 0/6] Patches for clang compilation Bill Wendling
` (4 preceding siblings ...)
2019-10-30 21:04 ` [kvm-unit-tests PATCH v3 5/6] x86: use a non-negative number in shift Bill Wendling
@ 2019-10-30 21:04 ` Bill Wendling
5 siblings, 0 replies; 37+ messages in thread
From: Bill Wendling @ 2019-10-30 21:04 UTC (permalink / raw)
To: kvm, pbonzini, thuth; +Cc: jmattson, sean.j.christopherson, Bill Wendling
According to GCC's documentation, the only supported use for specifying
registers for local variables is "to specify registers for input and
output operands when calling Extended asm." Using it as a shortcut to
get the value in a register isn't guaranteed to work, and clang
complains that the variable is uninitialized.
Signed-off-by: Bill Wendling <morbo@google.com>
---
x86/vmx_tests.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 4aebc3f..565b69b 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -2138,7 +2138,9 @@ static void into_guest_main(void)
.offset = (uintptr_t)&&into,
.selector = KERNEL_CS32,
};
- register uintptr_t rsp asm("rsp");
+ uintptr_t rsp;
+
+ asm volatile ("mov %%rsp, %0" : "=r"(rsp));
if (fp.offset != (uintptr_t)&&into) {
printf("Code address too high.\n");
@@ -3221,7 +3223,9 @@ static void try_compat_invvpid(void *unused)
.offset = (uintptr_t)&&invvpid,
.selector = KERNEL_CS32,
};
- register uintptr_t rsp asm("rsp");
+ uintptr_t rsp;
+
+ asm volatile ("mov %%rsp, %0" : "=r"(rsp));
TEST_ASSERT_MSG(fp.offset == (uintptr_t)&&invvpid,
"Code address too high.");
--
2.24.0.rc1.363.gb1bccd3e3d-goog
^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [kvm-unit-tests PATCH v3 5/6] x86: use a non-negative number in shift
2019-10-30 21:04 ` [kvm-unit-tests PATCH v3 5/6] x86: use a non-negative number in shift Bill Wendling
@ 2019-11-08 8:31 ` Thomas Huth
0 siblings, 0 replies; 37+ messages in thread
From: Thomas Huth @ 2019-11-08 8:31 UTC (permalink / raw)
To: Bill Wendling, kvm, pbonzini; +Cc: jmattson, sean.j.christopherson
On 30/10/2019 22.04, Bill Wendling wrote:
> Shifting a negative number is undefined. Clang complains about it:
>
> x86/svm.c:1131:38: error: shifting a negative signed value is undefined [-Werror,-Wshift-negative-value]
> test->vmcb->control.tsc_offset = TSC_OFFSET_VALUE;
>
> Using "~0ull" results in identical asm code:
>
> before: movabsq $-281474976710656, %rsi
> after: movabsq $-281474976710656, %rsi
>
> Signed-off-by: Bill Wendling <morbo@google.com>
> ---
> x86/svm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/x86/svm.c b/x86/svm.c
> index 4ddfaa4..cef43d5 100644
> --- a/x86/svm.c
> +++ b/x86/svm.c
> @@ -1122,7 +1122,7 @@ static bool npt_rw_l1mmio_check(struct test *test)
> }
>
> #define TSC_ADJUST_VALUE (1ll << 32)
> -#define TSC_OFFSET_VALUE (-1ll << 48)
> +#define TSC_OFFSET_VALUE (~0ull << 48)
> static bool ok;
>
> static void tsc_adjust_prepare(struct test *test)
>
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [kvm-unit-tests PATCH v3 2/6] pci: cast the masks to the appropriate size
2019-10-30 21:04 ` [kvm-unit-tests PATCH v3 2/6] pci: cast the masks to the appropriate size Bill Wendling
@ 2019-11-08 8:39 ` Thomas Huth
0 siblings, 0 replies; 37+ messages in thread
From: Thomas Huth @ 2019-11-08 8:39 UTC (permalink / raw)
To: Bill Wendling, kvm, pbonzini; +Cc: jmattson, sean.j.christopherson
On 30/10/2019 22.04, Bill Wendling wrote:
> At this point, we're dealing with 32-bit addresses, therefore downcast
> the masks to 32-bits.
In case you respin, please mention the warning from clang that you are
fixing here.
> diff --git a/lib/pci.c b/lib/pci.c
> index daa33e1..1b85411 100644
> --- a/lib/pci.c
> +++ b/lib/pci.c
> @@ -107,7 +107,8 @@ pcidevaddr_t pci_find_dev(uint16_t vendor_id, uint16_t device_id)
> uint32_t pci_bar_mask(uint32_t bar)
> {
> return (bar & PCI_BASE_ADDRESS_SPACE_IO) ?
> - PCI_BASE_ADDRESS_IO_MASK : PCI_BASE_ADDRESS_MEM_MASK;
> + (uint32_t)PCI_BASE_ADDRESS_IO_MASK :
> + (uint32_t)PCI_BASE_ADDRESS_MEM_MASK;
> }
>
> uint32_t pci_bar_get(struct pci_dev *dev, int bar_num)
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [kvm-unit-tests PATCH v3 3/6] Makefile: use "-Werror" in cc-option
2019-10-30 21:04 ` [kvm-unit-tests PATCH v3 3/6] Makefile: use "-Werror" in cc-option Bill Wendling
@ 2019-11-08 8:43 ` Thomas Huth
2019-11-08 20:36 ` Bill Wendling
0 siblings, 1 reply; 37+ messages in thread
From: Thomas Huth @ 2019-11-08 8:43 UTC (permalink / raw)
To: Bill Wendling, kvm, pbonzini; +Cc: jmattson, sean.j.christopherson
On 30/10/2019 22.04, Bill Wendling wrote:
> The "cc-option" macro should use "-Werror" to determine if a flag is
> supported. Otherwise the test may not return a nonzero result. Also
> conditionalize some of the warning flags which aren't supported by
> clang.
>
> Signed-off-by: Bill Wendling <morbo@google.com>
> ---
> Makefile | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 32414dc..6201c45 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -46,13 +46,13 @@ include $(SRCDIR)/$(TEST_DIR)/Makefile
> # cc-option
> # Usage: OP_CFLAGS+=$(call cc-option, -falign-functions=0, -malign-functions=0)
>
> -cc-option = $(shell if $(CC) $(1) -S -o /dev/null -xc /dev/null \
> +cc-option = $(shell if $(CC) -Werror $(1) -S -o /dev/null -xc /dev/null \
> > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;)
>
> COMMON_CFLAGS += -g $(autodepend-flags)
> -COMMON_CFLAGS += -Wall -Wwrite-strings -Wclobbered -Wempty-body -Wuninitialized
> -COMMON_CFLAGS += -Wignored-qualifiers -Wunused-but-set-parameter
> -COMMON_CFLAGS += -Werror
> +COMMON_CFLAGS += -Wall -Wwrite-strings -Wempty-body -Wuninitialized
> +COMMON_CFLAGS += -Wignored-qualifiers -Werror
> +
> frame-pointer-flag=-f$(if $(KEEP_FRAME_POINTER),no-,)omit-frame-pointer
> fomit_frame_pointer := $(call cc-option, $(frame-pointer-flag), "")
> fnostack_protector := $(call cc-option, -fno-stack-protector, "")
> @@ -60,16 +60,20 @@ fnostack_protector_all := $(call cc-option, -fno-stack-protector-all, "")
> wno_frame_address := $(call cc-option, -Wno-frame-address, "")
> fno_pic := $(call cc-option, -fno-pic, "")
> no_pie := $(call cc-option, -no-pie, "")
> +wclobbered := $(call cc-option, -Wclobbered, "")
> +wunused_but_set_parameter := $(call cc-option, -Wunused-but-set-parameter, "")
> +
> COMMON_CFLAGS += $(fomit_frame_pointer)
> COMMON_CFLAGS += $(fno_stack_protector)
> COMMON_CFLAGS += $(fno_stack_protector_all)
> COMMON_CFLAGS += $(wno_frame_address)
> COMMON_CFLAGS += $(if $(U32_LONG_FMT),-D__U32_LONG_FMT__,)
> COMMON_CFLAGS += $(fno_pic) $(no_pie)
> +COMMON_CFLAGS += $(wclobbered)
> +COMMON_CFLAGS += $(wunused_but_set_parameter)
Looks good to me up to this point here...
> CFLAGS += $(COMMON_CFLAGS)
> -CFLAGS += -Wmissing-parameter-type -Wold-style-declaration -Woverride-init
> -CFLAGS += -Wmissing-prototypes -Wstrict-prototypes
> +CFLAGS += -Woverride-init -Wmissing-prototypes -Wstrict-prototypes
... but I think this hunk rather belongs to the next patch instead?
Thomas
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [kvm-unit-tests PATCH v3 4/6] Makefile: add "cxx-option" for C++ builds
2019-10-30 21:04 ` [kvm-unit-tests PATCH v3 4/6] Makefile: add "cxx-option" for C++ builds Bill Wendling
@ 2019-11-08 8:48 ` Thomas Huth
0 siblings, 0 replies; 37+ messages in thread
From: Thomas Huth @ 2019-11-08 8:48 UTC (permalink / raw)
To: Bill Wendling, kvm, pbonzini; +Cc: jmattson, sean.j.christopherson
On 30/10/2019 22.04, Bill Wendling wrote:
> The C++ compiler may not support all of the same flags as the C
> compiler. Add a separate test for these flags.
>
> Signed-off-by: Bill Wendling <morbo@google.com>
> ---
> Makefile | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index 6201c45..9cb47e6 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -48,6 +48,8 @@ include $(SRCDIR)/$(TEST_DIR)/Makefile
>
> cc-option = $(shell if $(CC) -Werror $(1) -S -o /dev/null -xc /dev/null \
> > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;)
> +cxx-option = $(shell if $(CXX) -Werror $(1) -S -o /dev/null -xc++ /dev/null \
> + > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;)
>
> COMMON_CFLAGS += -g $(autodepend-flags)
> COMMON_CFLAGS += -Wall -Wwrite-strings -Wempty-body -Wuninitialized
> @@ -73,9 +75,19 @@ COMMON_CFLAGS += $(wclobbered)
> COMMON_CFLAGS += $(wunused_but_set_parameter)
>
> CFLAGS += $(COMMON_CFLAGS)
> +CXXFLAGS += $(COMMON_CFLAGS)
> +
> +wmissing_parameter_type := $(call cc-option, -Wmissing-parameter-type, "")
> +wold_style_declaration := $(call cc-option, -Wold-style-declaration, "")
> +CFLAGS += $(wmissing_parameter_type)
> +CFLAGS += $(wold_style_declaration)
> CFLAGS += -Woverride-init -Wmissing-prototypes -Wstrict-prototypes
>
> -CXXFLAGS += $(COMMON_CFLAGS)
> +# Clang's C++ compiler doesn't support some of the flags its C compiler does.
> +wmissing_parameter_type := $(call cxx-option, -Wmissing-parameter-type, "")
> +wold_style_declaration := $(call cxx-option, -Wold-style-declaration, "")
> +CXXFLAGS += $(wmissing_parameter_type)
> +CXXFLAGS += $(wold_style_declaration)
According to the man page of gcc-9, both options are for "C and
Objective-C only". I think you can simply always remove them from the
CXXFLAGS.
Thomas
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [kvm-unit-tests PATCH v3 3/6] Makefile: use "-Werror" in cc-option
2019-11-08 8:43 ` Thomas Huth
@ 2019-11-08 20:36 ` Bill Wendling
0 siblings, 0 replies; 37+ messages in thread
From: Bill Wendling @ 2019-11-08 20:36 UTC (permalink / raw)
To: Thomas Huth; +Cc: kvm list, Paolo Bonzini, Jim Mattson, Sean Christopherson
I sent out another set of patches for the -Werror, because I didn't
see the ones I posted on the list and thought they got lost.
On Fri, Nov 8, 2019 at 12:43 AM Thomas Huth <thuth@redhat.com> wrote:
>
> On 30/10/2019 22.04, Bill Wendling wrote:
> > The "cc-option" macro should use "-Werror" to determine if a flag is
> > supported. Otherwise the test may not return a nonzero result. Also
> > conditionalize some of the warning flags which aren't supported by
> > clang.
> >
> > Signed-off-by: Bill Wendling <morbo@google.com>
> > ---
> > Makefile | 16 ++++++++++------
> > 1 file changed, 10 insertions(+), 6 deletions(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 32414dc..6201c45 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -46,13 +46,13 @@ include $(SRCDIR)/$(TEST_DIR)/Makefile
> > # cc-option
> > # Usage: OP_CFLAGS+=$(call cc-option, -falign-functions=0, -malign-functions=0)
> >
> > -cc-option = $(shell if $(CC) $(1) -S -o /dev/null -xc /dev/null \
> > +cc-option = $(shell if $(CC) -Werror $(1) -S -o /dev/null -xc /dev/null \
> > > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;)
> >
> > COMMON_CFLAGS += -g $(autodepend-flags)
> > -COMMON_CFLAGS += -Wall -Wwrite-strings -Wclobbered -Wempty-body -Wuninitialized
> > -COMMON_CFLAGS += -Wignored-qualifiers -Wunused-but-set-parameter
> > -COMMON_CFLAGS += -Werror
> > +COMMON_CFLAGS += -Wall -Wwrite-strings -Wempty-body -Wuninitialized
> > +COMMON_CFLAGS += -Wignored-qualifiers -Werror
> > +
> > frame-pointer-flag=-f$(if $(KEEP_FRAME_POINTER),no-,)omit-frame-pointer
> > fomit_frame_pointer := $(call cc-option, $(frame-pointer-flag), "")
> > fnostack_protector := $(call cc-option, -fno-stack-protector, "")
> > @@ -60,16 +60,20 @@ fnostack_protector_all := $(call cc-option, -fno-stack-protector-all, "")
> > wno_frame_address := $(call cc-option, -Wno-frame-address, "")
> > fno_pic := $(call cc-option, -fno-pic, "")
> > no_pie := $(call cc-option, -no-pie, "")
> > +wclobbered := $(call cc-option, -Wclobbered, "")
> > +wunused_but_set_parameter := $(call cc-option, -Wunused-but-set-parameter, "")
> > +
> > COMMON_CFLAGS += $(fomit_frame_pointer)
> > COMMON_CFLAGS += $(fno_stack_protector)
> > COMMON_CFLAGS += $(fno_stack_protector_all)
> > COMMON_CFLAGS += $(wno_frame_address)
> > COMMON_CFLAGS += $(if $(U32_LONG_FMT),-D__U32_LONG_FMT__,)
> > COMMON_CFLAGS += $(fno_pic) $(no_pie)
> > +COMMON_CFLAGS += $(wclobbered)
> > +COMMON_CFLAGS += $(wunused_but_set_parameter)
>
> Looks good to me up to this point here...
>
> > CFLAGS += $(COMMON_CFLAGS)
> > -CFLAGS += -Wmissing-parameter-type -Wold-style-declaration -Woverride-init
> > -CFLAGS += -Wmissing-prototypes -Wstrict-prototypes
> > +CFLAGS += -Woverride-init -Wmissing-prototypes -Wstrict-prototypes
>
> ... but I think this hunk rather belongs to the next patch instead?
>
> Thomas
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [kvm-unit-tests PATCH v3 1/6] x86: emulator: use "SSE2" for the target
2019-10-30 21:04 ` [kvm-unit-tests PATCH v3 1/6] x86: emulator: use "SSE2" for the target Bill Wendling
@ 2019-11-28 7:31 ` Thomas Huth
0 siblings, 0 replies; 37+ messages in thread
From: Thomas Huth @ 2019-11-28 7:31 UTC (permalink / raw)
To: Bill Wendling, kvm, pbonzini; +Cc: jmattson, sean.j.christopherson
On 30/10/2019 22.04, Bill Wendling wrote:
> The movdqu and movapd instructions are SSE2 instructions. Clang
> interprets the __attribute__((target("sse"))) as allowing SSE only
> instructions. Using SSE2 instructions cause an error.
>
> Signed-off-by: Bill Wendling <morbo@google.com>
> ---
> x86/emulator.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/x86/emulator.c b/x86/emulator.c
> index 621caf9..bec0154 100644
> --- a/x86/emulator.c
> +++ b/x86/emulator.c
> @@ -657,7 +657,7 @@ static bool sseeq(sse_union *v1, sse_union *v2)
> return ok;
> }
>
> -static __attribute__((target("sse"))) void test_sse(sse_union *mem)
> +static __attribute__((target("sse2"))) void test_sse(sse_union *mem)
> {
> sse_union v;
That seems to work fine with both, gcc and clang, thus:
Tested-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 37+ messages in thread
end of thread, other threads:[~2019-11-28 7:31 UTC | newest]
Thread overview: 37+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-10 18:35 [kvm-unit-tests PATCH 0/3] Patches for clang compilation Bill Wendling
2019-10-10 18:35 ` [kvm-unit-tests PATCH 1/3] x86: emulator: use "SSE2" for the target Bill Wendling
2019-10-10 18:35 ` [kvm-unit-tests PATCH 2/3] pci: use uint64_t for unsigned long values Bill Wendling
2019-10-11 9:12 ` Alexandru Elisei
2019-10-12 8:26 ` [kvm-unit-tests PATCH 1/1] " Bill Wendling
2019-10-14 16:33 ` Alexandru Elisei
2019-10-14 16:36 ` Sean Christopherson
2019-10-10 18:35 ` [kvm-unit-tests PATCH 3/3] Makefile: use "-Werror" in cc-option Bill Wendling
2019-10-15 7:29 ` Thomas Huth
2019-10-15 7:57 ` Bill Wendling
2019-10-15 8:12 ` Thomas Huth
2019-10-14 19:24 ` [kvm-unit-tests PATCH 0/4] Patches for clang compilation Bill Wendling
2019-10-14 19:24 ` [kvm-unit-tests PATCH 1/4] x86: emulator: use "SSE2" for the target Bill Wendling
2019-10-14 19:24 ` [kvm-unit-tests PATCH 2/4] pci: cast the masks to the appropriate size Bill Wendling
2019-10-14 19:24 ` [kvm-unit-tests PATCH 3/4] Makefile: use "-Werror" in cc-option Bill Wendling
2019-10-14 19:24 ` [kvm-unit-tests PATCH 4/4] Makefile: add "cxx-option" for C++ builds Bill Wendling
2019-10-14 19:25 ` [kvm-unit-tests PATCH 0/4] Patches for clang compilation Bill Wendling
2019-10-14 23:56 ` Nadav Amit
2019-10-15 0:04 ` Bill Wendling
2019-10-15 0:04 ` [kvm-unit-tests PATCH v2 " Bill Wendling
2019-10-15 0:04 ` [kvm-unit-tests PATCH v2 1/4] x86: emulator: use "SSE2" for the target Bill Wendling
2019-10-15 0:04 ` [kvm-unit-tests PATCH v2 2/4] pci: cast the masks to the appropriate size Bill Wendling
2019-10-15 0:04 ` [kvm-unit-tests PATCH v2 3/4] Makefile: use "-Werror" in cc-option Bill Wendling
2019-10-15 0:04 ` [kvm-unit-tests PATCH v2 4/4] Makefile: add "cxx-option" for C++ builds Bill Wendling
2019-10-30 21:04 ` [kvm-unit-tests PATCH v3 0/6] Patches for clang compilation Bill Wendling
2019-10-30 21:04 ` [kvm-unit-tests PATCH v3 1/6] x86: emulator: use "SSE2" for the target Bill Wendling
2019-11-28 7:31 ` Thomas Huth
2019-10-30 21:04 ` [kvm-unit-tests PATCH v3 2/6] pci: cast the masks to the appropriate size Bill Wendling
2019-11-08 8:39 ` Thomas Huth
2019-10-30 21:04 ` [kvm-unit-tests PATCH v3 3/6] Makefile: use "-Werror" in cc-option Bill Wendling
2019-11-08 8:43 ` Thomas Huth
2019-11-08 20:36 ` Bill Wendling
2019-10-30 21:04 ` [kvm-unit-tests PATCH v3 4/6] Makefile: add "cxx-option" for C++ builds Bill Wendling
2019-11-08 8:48 ` Thomas Huth
2019-10-30 21:04 ` [kvm-unit-tests PATCH v3 5/6] x86: use a non-negative number in shift Bill Wendling
2019-11-08 8:31 ` Thomas Huth
2019-10-30 21:04 ` [kvm-unit-tests PATCH v3 6/6] x86: use inline asm to retrieve stack pointer Bill Wendling
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).