[kvm-unit-tests PATCH v8 00/12] Cleanup low-level arch code

[kvm-unit-tests PATCH v8 00/12] Cleanup low-level arch code

[Alexander Gordeev]

Hi all,

This is 8th attempt to make mainly x86 arch code consistent
with other architectures. This series is a prerequisite for
a follow-up "PCI bus support" series which I am going to
send separately.


Changes since v7:
  - PT_NX_MASK macro renamed to PT64_NX_MASK;
  - added patch "io: Unify IO accessors across architectures";

Changes since v6:
  - ioremap() PCD comment updated;
  - PT_PSE_MASK renamed to PT_PAGE_SIZE_MASK;
  - GENMASK_ULL used for PT_ADDR_MASK definition;
  - missed x86/taskswitch2.c update added;

Changes since v5:
  - ioremap() PCD comment added, the Reviewed-by preserved;
  - page table flag names changed from PTE_* to PT_*_MASK;

Changes since v4:
  - single instead of multiple pages allocation bug fixed in x86 ioremap();
  - style fixes squashed into code movements for lib/x86/asm/page.h and
    lib/x86/asm/barrier.h;
  - #ifndef __ASSEMBLY__ added to lib/x86/asm/page.h;

Changes since v3:
  - x86 PAGE_* constants and PTE_* flags cleaned up;
  - missing Reviewed-bys restored;
  - missing headers to lib/x86/asm/page.h and lib/x86/asm/barriers.h added
    (I did not remove already given Reviewed-bys from these);

Changes since v2:
  - x86 ioremap() address arithmetics nonsense fixed;
  - x86 virt_to_phys() and phys_to_virt() implementations left intact;
  - separate overrides of virt_to_phys() and phys_to_virt() added;
  - unsigned long size specifier removed from PAGE_SIZE constant;

Changes since v1:
  - arm compilation error fixed;
  - disabling memory re-ordering for generic memory barriers added;

Cc: Andrew Jones <drjones@redhat.com>
Cc: Thomas Huth <thuth@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>

Alexander Gordeev (12):
  Remove unused and unnecessary PHYS32 macro
  Move phys_addr_t type definition to lib/libcflat.h
  x86: Introduce lib/x86/asm/page.h
  x86: Introduce lib/x86/asm/io.h
  x86: Introduce lib/x86/asm/barrier.h
  io: Separate overrides of virt_to_phys() and phys_to_virt()
  io: Disallow memory re-ordering for generic memory barriers
  x86: Cleanup PAGE_* constants
  x86: Cleanup PT_*_MASK flags
  io: Make ioremap() prototype conform to Linux one
  x86: io: Factor out ioremap()
  io: Unify IO accessors across architectures

 arm/selftest.c            |  1 -
 lib/alloc.h               |  5 ----
 lib/arm/asm/page.h        |  2 --
 lib/asm-generic/barrier.h | 21 +++++++++++++++
 lib/asm-generic/io.h      | 56 +++++++++++++++++++++++++++++++++-------
 lib/libcflat.h            |  2 ++
 lib/powerpc/asm/setup.h   |  1 -
 lib/ppc64/asm/barrier.h   |  4 +++
 lib/ppc64/asm/io.h        |  2 ++
 lib/x86/asm/barrier.h     | 13 ++++++++++
 lib/x86/asm/io.h          | 65 +++++++++++++++++++++++++++++++++++++++++++++++
 lib/x86/asm/page.h        | 45 ++++++++++++++++++++++++++++++++
 lib/x86/asm/pci.h         |  2 +-
 lib/x86/io.c              | 18 ++++++++++++-
 lib/x86/io.h              | 40 -----------------------------
 lib/x86/smp.h             |  4 ---
 lib/x86/vm.c              | 37 ++++++++-------------------
 lib/x86/vm.h              | 25 ++----------------
 x86/access.c              | 18 +++----------
 x86/asyncpf.c             |  2 +-
 x86/eventinj.c            | 17 +++++--------
 x86/hypercall.c           |  2 +-
 x86/hyperv.c              |  1 +
 x86/hyperv.h              |  1 -
 x86/hyperv_stimer.c       |  2 +-
 x86/hyperv_synic.c        |  1 -
 x86/init.c                |  2 +-
 x86/kvmclock.c            |  1 +
 x86/pku.c                 |  2 +-
 x86/rmap_chain.c          |  2 +-
 x86/smap.c                |  6 ++---
 x86/svm.c                 |  1 -
 x86/taskswitch2.c         |  2 +-
 x86/vmexit.c              | 11 ++------
 x86/vmx.c                 |  1 -
 x86/vmx.h                 |  2 +-
 x86/vmx_tests.c           |  1 -
 37 files changed, 254 insertions(+), 164 deletions(-)
 create mode 100644 lib/asm-generic/barrier.h
 create mode 100644 lib/ppc64/asm/barrier.h
 create mode 100644 lib/x86/asm/barrier.h
 create mode 100644 lib/x86/asm/io.h
 create mode 100644 lib/x86/asm/page.h
 delete mode 100644 lib/x86/io.h

--
1.8.3.1

[kvm-unit-tests PATCH v8 01/12] Remove unused and unnecessary PHYS32 macro

[Alexander Gordeev]

Cc: Andrew Jones <drjones@redhat.com>
Cc: Thomas Huth <thuth@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Suggested-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
 lib/alloc.h | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/lib/alloc.h b/lib/alloc.h
index 7a73c18bef97..0b4b4bdc35ba 100644
--- a/lib/alloc.h
+++ b/lib/alloc.h
@@ -58,11 +58,7 @@ static inline void *memalign(size_t alignment, size_t size)
 	return alloc_ops->memalign(alignment, size);
 }
 
-#ifdef PHYS32
-typedef u32 phys_addr_t;
-#else
 typedef u64 phys_addr_t;
-#endif
 #define INVALID_PHYS_ADDR (~(phys_addr_t)0)
 
 /*
-- 
1.8.3.1

[kvm-unit-tests PATCH v8 02/12] Move phys_addr_t type definition to lib/libcflat.h

[Alexander Gordeev]

This change leads to removing '#include <alloc.h>'s from
several places that only included alloc.h to get the typedef.

Cc: Andrew Jones <drjones@redhat.com>
Cc: Thomas Huth <thuth@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Suggested-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
 arm/selftest.c          | 1 -
 lib/alloc.h             | 1 -
 lib/arm/asm/page.h      | 2 --
 lib/libcflat.h          | 2 ++
 lib/powerpc/asm/setup.h | 1 -
 5 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/arm/selftest.c b/arm/selftest.c
index 75dc91faab69..a8ae191d3201 100644
--- a/arm/selftest.c
+++ b/arm/selftest.c
@@ -7,7 +7,6 @@
  */
 #include <libcflat.h>
 #include <util.h>
-#include <alloc.h>
 #include <devicetree.h>
 #include <asm/setup.h>
 #include <asm/ptrace.h>
diff --git a/lib/alloc.h b/lib/alloc.h
index 0b4b4bdc35ba..c12bd15f7afc 100644
--- a/lib/alloc.h
+++ b/lib/alloc.h
@@ -58,7 +58,6 @@ static inline void *memalign(size_t alignment, size_t size)
 	return alloc_ops->memalign(alignment, size);
 }
 
-typedef u64 phys_addr_t;
 #define INVALID_PHYS_ADDR (~(phys_addr_t)0)
 
 /*
diff --git a/lib/arm/asm/page.h b/lib/arm/asm/page.h
index df76969964ed..38026418edba 100644
--- a/lib/arm/asm/page.h
+++ b/lib/arm/asm/page.h
@@ -16,8 +16,6 @@
 
 #define PAGE_ALIGN(addr)	ALIGN(addr, PAGE_SIZE)
 
-#include <alloc.h>
-
 typedef u64 pteval_t;
 typedef u64 pmdval_t;
 typedef u64 pgdval_t;
diff --git a/lib/libcflat.h b/lib/libcflat.h
index 582e3fc60e28..72b1bf9668ef 100644
--- a/lib/libcflat.h
+++ b/lib/libcflat.h
@@ -60,6 +60,8 @@ typedef _Bool		bool;
 #define PRIx64  __PRI64_PREFIX	"x"
 #define PRIxPTR __PRIPTR_PREFIX	"x"
 
+typedef u64			phys_addr_t;
+
 extern void puts(const char *s);
 extern void exit(int code);
 extern void abort(void);
diff --git a/lib/powerpc/asm/setup.h b/lib/powerpc/asm/setup.h
index 29a6d7d5f705..b1e1e5a51ed3 100644
--- a/lib/powerpc/asm/setup.h
+++ b/lib/powerpc/asm/setup.h
@@ -6,7 +6,6 @@
  * This work is licensed under the terms of the GNU LGPL, version 2.
  */
 #include <libcflat.h>
-#include <alloc.h>	/* phys_addr_t */
 
 #define NR_CPUS			8	/* arbitrarily set for now */
 extern u32 cpus[NR_CPUS];
-- 
1.8.3.1

[kvm-unit-tests PATCH v8 03/12] x86: Introduce lib/x86/asm/page.h

[Alexander Gordeev]

Make x86 consistent with other architectures and put
memory page specific defines to lib/x86/asm/page.h

Cc: Andrew Jones <drjones@redhat.com>
Cc: Thomas Huth <thuth@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
 lib/x86/asm/page.h | 33 +++++++++++++++++++++++++++++++++
 lib/x86/vm.c       | 17 -----------------
 lib/x86/vm.h       | 14 +-------------
 3 files changed, 34 insertions(+), 30 deletions(-)
 create mode 100644 lib/x86/asm/page.h

diff --git a/lib/x86/asm/page.h b/lib/x86/asm/page.h
new file mode 100644
index 000000000000..d4580b40f5e8
--- /dev/null
+++ b/lib/x86/asm/page.h
@@ -0,0 +1,33 @@
+#ifndef _ASM_X86_PAGE_H_
+#define _ASM_X86_PAGE_H_
+/*
+ * Copyright (C) 2016, Red Hat Inc, Alexander Gordeev <agordeev@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.
+ */
+
+
+#define PAGE_SIZE	4096ul
+#ifdef __x86_64__
+#define LARGE_PAGE_SIZE	(512 * PAGE_SIZE)
+#else
+#define LARGE_PAGE_SIZE	(1024 * PAGE_SIZE)
+#endif
+
+#define PTE_PRESENT	(1ull << 0)
+#define PTE_WRITE	(1ull << 1)
+#define PTE_USER	(1ull << 2)
+#define PTE_PSE		(1ull << 7)
+#define PTE_ADDR	(0xffffffffff000ull)
+
+#ifdef __x86_64__
+#define	PAGE_LEVEL	4
+#define	PGDIR_WIDTH	9
+#define	PGDIR_MASK	511
+#else
+#define	PAGE_LEVEL	2
+#define	PGDIR_WIDTH	10
+#define	PGDIR_MASK	1023
+#endif
+
+#endif
diff --git a/lib/x86/vm.c b/lib/x86/vm.c
index 7ce7bbc03eef..9c94ca5b7fc4 100644
--- a/lib/x86/vm.c
+++ b/lib/x86/vm.c
@@ -2,13 +2,6 @@
 #include "vm.h"
 #include "libcflat.h"
 
-#define PAGE_SIZE 4096ul
-#ifdef __x86_64__
-#define LARGE_PAGE_SIZE (512 * PAGE_SIZE)
-#else
-#define LARGE_PAGE_SIZE (1024 * PAGE_SIZE)
-#endif
-
 static void *free = 0;
 static void *vfree_top = 0;
 
@@ -44,16 +37,6 @@ void free_page(void *page)
 extern char edata;
 static unsigned long end_of_memory;
 
-#ifdef __x86_64__
-#define	PAGE_LEVEL	4
-#define	PGDIR_WIDTH	9
-#define	PGDIR_MASK	511
-#else
-#define	PAGE_LEVEL	2
-#define	PGDIR_WIDTH	10
-#define	PGDIR_MASK	1023
-#endif
-
 unsigned long *install_pte(unsigned long *cr3,
 			   int pte_level,
 			   void *virt,
diff --git a/lib/x86/vm.h b/lib/x86/vm.h
index 28794d7f26c6..72f84e6f5544 100644
--- a/lib/x86/vm.h
+++ b/lib/x86/vm.h
@@ -2,19 +2,7 @@
 #define VM_H
 
 #include "processor.h"
-
-#define PAGE_SIZE 4096ul
-#ifdef __x86_64__
-#define LARGE_PAGE_SIZE (512 * PAGE_SIZE)
-#else
-#define LARGE_PAGE_SIZE (1024 * PAGE_SIZE)
-#endif
-
-#define PTE_PRESENT (1ull << 0)
-#define PTE_PSE     (1ull << 7)
-#define PTE_WRITE   (1ull << 1)
-#define PTE_USER    (1ull << 2)
-#define PTE_ADDR    (0xffffffffff000ull)
+#include "asm/page.h"
 
 void setup_vm();
 
-- 
1.8.3.1

[kvm-unit-tests PATCH v8 04/12] x86: Introduce lib/x86/asm/io.h

[Alexander Gordeev]

Make x86 consistent with other architectures and put
IO specific defines to lib/x86/asm/io.h

Cc: Andrew Jones <drjones@redhat.com>
Cc: Thomas Huth <thuth@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
 lib/x86/asm/io.h    | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/x86/asm/pci.h   |  2 +-
 lib/x86/io.c        |  2 +-
 lib/x86/io.h        | 40 ----------------------------------------
 lib/x86/vm.h        | 11 +----------
 x86/eventinj.c      |  7 +------
 x86/hyperv.c        |  1 +
 x86/hyperv.h        |  1 -
 x86/hyperv_stimer.c |  1 -
 x86/hyperv_synic.c  |  1 -
 x86/init.c          |  2 +-
 x86/svm.c           |  1 -
 x86/vmexit.c        |  1 -
 x86/vmx.c           |  1 -
 x86/vmx_tests.c     |  1 -
 15 files changed, 56 insertions(+), 66 deletions(-)
 create mode 100644 lib/x86/asm/io.h
 delete mode 100644 lib/x86/io.h

diff --git a/lib/x86/asm/io.h b/lib/x86/asm/io.h
new file mode 100644
index 000000000000..c944df4b07c8
--- /dev/null
+++ b/lib/x86/asm/io.h
@@ -0,0 +1,50 @@
+#ifndef _ASM_X86_IO_H_
+#define _ASM_X86_IO_H_
+
+static inline unsigned char inb(unsigned short port)
+{
+    unsigned char value;
+    asm volatile("inb %w1, %0" : "=a" (value) : "Nd" (port));
+    return value;
+}
+
+static inline unsigned short inw(unsigned short port)
+{
+    unsigned short value;
+    asm volatile("inw %w1, %0" : "=a" (value) : "Nd" (port));
+    return value;
+}
+
+static inline unsigned int inl(unsigned short port)
+{
+    unsigned int value;
+    asm volatile("inl %w1, %0" : "=a" (value) : "Nd" (port));
+    return value;
+}
+
+static inline void outb(unsigned char value, unsigned short port)
+{
+    asm volatile("outb %b0, %w1" : : "a"(value), "Nd"(port));
+}
+
+static inline void outw(unsigned short value, unsigned short port)
+{
+    asm volatile("outw %w0, %w1" : : "a"(value), "Nd"(port));
+}
+
+static inline void outl(unsigned int value, unsigned short port)
+{
+    asm volatile("outl %0, %w1" : : "a"(value), "Nd"(port));
+}
+
+static inline unsigned long virt_to_phys(const void *virt)
+{
+    return (unsigned long)virt;
+}
+
+static inline void *phys_to_virt(unsigned long phys)
+{
+    return (void *)phys;
+}
+
+#endif
diff --git a/lib/x86/asm/pci.h b/lib/x86/asm/pci.h
index 4ec20e17d25b..cddde41a7d97 100644
--- a/lib/x86/asm/pci.h
+++ b/lib/x86/asm/pci.h
@@ -7,7 +7,7 @@
  */
 #include "libcflat.h"
 #include "pci.h"
-#include "x86/io.h"
+#include "x86/asm/io.h"
 
 static inline uint32_t pci_config_read(pcidevaddr_t dev, uint8_t reg)
 {
diff --git a/lib/x86/io.c b/lib/x86/io.c
index d3b971ef67b0..d396d42d0535 100644
--- a/lib/x86/io.c
+++ b/lib/x86/io.c
@@ -1,6 +1,6 @@
 #include "libcflat.h"
 #include "smp.h"
-#include "io.h"
+#include "asm/io.h"
 #ifndef USE_SERIAL
 #define USE_SERIAL
 #endif
diff --git a/lib/x86/io.h b/lib/x86/io.h
deleted file mode 100644
index bd6341c6c103..000000000000
--- a/lib/x86/io.h
+++ /dev/null
@@ -1,40 +0,0 @@
-#ifndef IO_H
-#define IO_H
-
-static inline unsigned char inb(unsigned short port)
-{
-    unsigned char value;
-    asm volatile("inb %w1, %0" : "=a" (value) : "Nd" (port));
-    return value;
-}
-
-static inline unsigned short inw(unsigned short port)
-{
-    unsigned short value;
-    asm volatile("inw %w1, %0" : "=a" (value) : "Nd" (port));
-    return value;
-}
-
-static inline unsigned int inl(unsigned short port)
-{
-    unsigned int value;
-    asm volatile("inl %w1, %0" : "=a" (value) : "Nd" (port));
-    return value;
-}
-
-static inline void outb(unsigned char value, unsigned short port)
-{
-    asm volatile("outb %b0, %w1" : : "a"(value), "Nd"(port));
-}
-
-static inline void outw(unsigned short value, unsigned short port)
-{
-    asm volatile("outw %w0, %w1" : : "a"(value), "Nd"(port));
-}
-
-static inline void outl(unsigned int value, unsigned short port)
-{
-    asm volatile("outl %0, %w1" : : "a"(value), "Nd"(port));
-}
-
-#endif
diff --git a/lib/x86/vm.h b/lib/x86/vm.h
index 72f84e6f5544..6a4384f5a48d 100644
--- a/lib/x86/vm.h
+++ b/lib/x86/vm.h
@@ -3,6 +3,7 @@
 
 #include "processor.h"
 #include "asm/page.h"
+#include "asm/io.h"
 
 void setup_vm();
 
@@ -27,14 +28,4 @@ unsigned long *install_large_page(unsigned long *cr3,unsigned long phys,
                                   void *virt);
 unsigned long *install_page(unsigned long *cr3, unsigned long phys, void *virt);
 
-static inline unsigned long virt_to_phys(const void *virt)
-{
-    return (unsigned long)virt;
-}
-
-static inline void *phys_to_virt(unsigned long phys)
-{
-    return (void *)phys;
-}
-
 #endif
diff --git a/x86/eventinj.c b/x86/eventinj.c
index 57c2a2d0cf28..84dfe71d8e2a 100644
--- a/x86/eventinj.c
+++ b/x86/eventinj.c
@@ -16,11 +16,6 @@ static inline void io_delay(void)
 {
 }
 
-static inline void outl(int addr, int val)
-{
-        asm volatile ("outl %1, %w0" : : "d" (addr), "a" (val));
-}
-
 void apic_self_ipi(u8 v)
 {
 	apic_icr_write(APIC_DEST_SELF | APIC_DEST_PHYSICAL | APIC_DM_FIXED |
@@ -32,7 +27,7 @@ void apic_self_nmi(void)
 	apic_icr_write(APIC_DEST_PHYSICAL | APIC_DM_NMI | APIC_INT_ASSERT, 0);
 }
 
-#define flush_phys_addr(__s) outl(0xe4, __s)
+#define flush_phys_addr(__s) outl(__s, 0xe4)
 #define flush_stack() do {						\
 		int __l;						\
 		flush_phys_addr(virt_to_phys(&__l));			\
diff --git a/x86/hyperv.c b/x86/hyperv.c
index 824773d07d67..2511aa2e24b3 100644
--- a/x86/hyperv.c
+++ b/x86/hyperv.c
@@ -1,4 +1,5 @@
 #include "hyperv.h"
+#include "asm/io.h"
 
 static void synic_ctl(u8 ctl, u8 vcpu_id, u8 sint)
 {
diff --git a/x86/hyperv.h b/x86/hyperv.h
index faf931bb4373..434a9334e2fd 100644
--- a/x86/hyperv.h
+++ b/x86/hyperv.h
@@ -3,7 +3,6 @@
 
 #include "libcflat.h"
 #include "processor.h"
-#include "io.h"
 
 #define HYPERV_CPUID_FEATURES                   0x40000003
 
diff --git a/x86/hyperv_stimer.c b/x86/hyperv_stimer.c
index bf2e4294ddad..9a971ef3593a 100644
--- a/x86/hyperv_stimer.c
+++ b/x86/hyperv_stimer.c
@@ -5,7 +5,6 @@
 #include "vm.h"
 #include "apic.h"
 #include "desc.h"
-#include "io.h"
 #include "smp.h"
 #include "atomic.h"
 #include "hyperv.h"
diff --git a/x86/hyperv_synic.c b/x86/hyperv_synic.c
index 5a06fb724a87..3f98c4280ef0 100644
--- a/x86/hyperv_synic.c
+++ b/x86/hyperv_synic.c
@@ -5,7 +5,6 @@
 #include "vm.h"
 #include "apic.h"
 #include "desc.h"
-#include "io.h"
 #include "smp.h"
 #include "atomic.h"
 #include "hyperv.h"
diff --git a/x86/init.c b/x86/init.c
index 344dc1c0b234..f47d671e62a8 100644
--- a/x86/init.c
+++ b/x86/init.c
@@ -1,6 +1,6 @@
 #include "libcflat.h"
 #include "apic.h"
-#include "io.h"
+#include "asm/io.h"
 
 #define KBD_CCMD_READ_OUTPORT   0xD0    /* read output port */
 #define KBD_CCMD_WRITE_OUTPORT  0xD1    /* write output port */
diff --git a/x86/svm.c b/x86/svm.c
index 301cf8cb2d6b..cef0f568d611 100644
--- a/x86/svm.c
+++ b/x86/svm.c
@@ -6,7 +6,6 @@
 #include "vm.h"
 #include "smp.h"
 #include "types.h"
-#include "io.h"
 
 /* for the nested page table*/
 u64 *pml4e;
diff --git a/x86/vmexit.c b/x86/vmexit.c
index 9e049752f6c6..db7dbd88fe2a 100644
--- a/x86/vmexit.c
+++ b/x86/vmexit.c
@@ -6,7 +6,6 @@
 #include "x86/vm.h"
 #include "x86/desc.h"
 #include "x86/acpi.h"
-#include "x86/io.h"
 
 struct test {
 	void (*func)(void);
diff --git a/x86/vmx.c b/x86/vmx.c
index 6618008e3d13..411ed3211d4d 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -35,7 +35,6 @@
 #include "vmx.h"
 #include "msr.h"
 #include "smp.h"
-#include "io.h"
 
 u64 *vmxon_region;
 struct vmcs *vmcs_root;
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 71c571cd661b..e83c8a26d34c 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -7,7 +7,6 @@
 #include "msr.h"
 #include "processor.h"
 #include "vm.h"
-#include "io.h"
 #include "fwcfg.h"
 #include "isr.h"
 #include "apic.h"
-- 
1.8.3.1

[kvm-unit-tests PATCH v8 05/12] x86: Introduce lib/x86/asm/barrier.h

[Alexander Gordeev]

Make x86 consistent with other architectures and put
memory barrier defines to lib/x86/asm/barrier.h

Cc: Andrew Jones <drjones@redhat.com>
Cc: Thomas Huth <thuth@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
 lib/x86/asm/barrier.h | 13 +++++++++++++
 lib/x86/smp.h         |  4 ----
 x86/hyperv_stimer.c   |  1 +
 x86/kvmclock.c        |  1 +
 4 files changed, 15 insertions(+), 4 deletions(-)
 create mode 100644 lib/x86/asm/barrier.h

diff --git a/lib/x86/asm/barrier.h b/lib/x86/asm/barrier.h
new file mode 100644
index 000000000000..d602a2c2d5bc
--- /dev/null
+++ b/lib/x86/asm/barrier.h
@@ -0,0 +1,13 @@
+#ifndef _ASM_X86_BARRIER_H_
+#define _ASM_X86_BARRIER_H_
+/*
+ * Copyright (C) 2016, Red Hat Inc, Alexander Gordeev <agordeev@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.
+ */
+
+#define mb()	asm volatile("mfence":::"memory")
+#define rmb()	asm volatile("lfence":::"memory")
+#define wmb()	asm volatile("sfence":::"memory")
+
+#endif
diff --git a/lib/x86/smp.h b/lib/x86/smp.h
index 566018f49ba3..afabac8495f1 100644
--- a/lib/x86/smp.h
+++ b/lib/x86/smp.h
@@ -2,10 +2,6 @@
 #define __SMP_H
 #include <asm/spinlock.h>
 
-#define mb() 	asm volatile("mfence":::"memory")
-#define rmb()	asm volatile("lfence":::"memory")
-#define wmb()	asm volatile("sfence" ::: "memory")
-
 void smp_init(void);
 
 int cpu_count(void);
diff --git a/x86/hyperv_stimer.c b/x86/hyperv_stimer.c
index 9a971ef3593a..63829388ae6e 100644
--- a/x86/hyperv_stimer.c
+++ b/x86/hyperv_stimer.c
@@ -8,6 +8,7 @@
 #include "smp.h"
 #include "atomic.h"
 #include "hyperv.h"
+#include "asm/barrier.h"
 
 #define MAX_CPUS 4
 
diff --git a/x86/kvmclock.c b/x86/kvmclock.c
index 327e60d34213..208d43cafc1e 100644
--- a/x86/kvmclock.c
+++ b/x86/kvmclock.c
@@ -3,6 +3,7 @@
 #include "atomic.h"
 #include "processor.h"
 #include "kvmclock.h"
+#include "asm/barrier.h"
 
 #define unlikely(x)	__builtin_expect(!!(x), 0)
 #define likely(x)	__builtin_expect(!!(x), 1)
-- 
1.8.3.1

[kvm-unit-tests PATCH v8 06/12] io: Separate overrides of virt_to_phys() and phys_to_virt()

[Alexander Gordeev]

Generic implementations of virt_to_phys() and phys_to_virt()
are currently covered by a single "virt_to_phys" macro.
Introduce additional macro "phys_to_virt" to allow separate
overrides.

Cc: Andrew Jones <drjones@redhat.com>
Cc: Thomas Huth <thuth@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
 lib/asm-generic/io.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/asm-generic/io.h b/lib/asm-generic/io.h
index 931415a465b7..3585ac09fb11 100644
--- a/lib/asm-generic/io.h
+++ b/lib/asm-generic/io.h
@@ -165,7 +165,9 @@ static inline unsigned long virt_to_phys(volatile void *address)
 {
 	return __pa((unsigned long)address);
 }
+#endif
 
+#ifndef phys_to_virt
 static inline void *phys_to_virt(unsigned long address)
 {
 	return __va(address);
-- 
1.8.3.1

[kvm-unit-tests PATCH v8 07/12] io: Disallow memory re-ordering for generic memory barriers

[Alexander Gordeev]

Suggested-by: Thomas Huth <thuth@redhat.com>
Cc: Andrew Jones <drjones@redhat.com>
Cc: Thomas Huth <thuth@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
 lib/asm-generic/io.h | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/lib/asm-generic/io.h b/lib/asm-generic/io.h
index 3585ac09fb11..1834720db2ac 100644
--- a/lib/asm-generic/io.h
+++ b/lib/asm-generic/io.h
@@ -127,11 +127,14 @@ static inline u64 __bswap64(u64 x)
 	({ u64 __r = !__cpu_is_be() ? __bswap64(x) : ((u64)x); __r; })
 #define cpu_to_be64 be64_to_cpu
 
+#ifndef mb
+#define mb()	asm volatile("":::"memory")
+#endif
 #ifndef rmb
-#define rmb() do { } while (0)
+#define rmb()	asm volatile("":::"memory")
 #endif
 #ifndef wmb
-#define wmb() do { } while (0)
+#define wmb()	asm volatile("":::"memory")
 #endif
 
 #define readb(addr) \
-- 
1.8.3.1

[kvm-unit-tests PATCH v8 08/12] x86: Cleanup PAGE_* constants

[Alexander Gordeev]

Cc: Andrew Jones <drjones@redhat.com>
Cc: Thomas Huth <thuth@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
 lib/x86/asm/page.h | 10 +++++++++-
 x86/access.c       |  4 +---
 x86/vmx.h          |  2 +-
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/lib/x86/asm/page.h b/lib/x86/asm/page.h
index d4580b40f5e8..916462d758ac 100644
--- a/lib/x86/asm/page.h
+++ b/lib/x86/asm/page.h
@@ -7,7 +7,14 @@
  */
 
 
-#define PAGE_SIZE	4096ul
+#include <linux/const.h>
+
+#define PAGE_SHIFT	12
+#define PAGE_SIZE	(_AC(1,UL) << PAGE_SHIFT)
+#define PAGE_MASK	(~(PAGE_SIZE-1))
+
+#ifndef __ASSEMBLY__
+
 #ifdef __x86_64__
 #define LARGE_PAGE_SIZE	(512 * PAGE_SIZE)
 #else
@@ -30,4 +37,5 @@
 #define	PGDIR_MASK	1023
 #endif
 
+#endif /* !__ASSEMBLY__ */
 #endif
diff --git a/x86/access.c b/x86/access.c
index 4acaee5c6b4d..aaf805d1ea7c 100644
--- a/x86/access.c
+++ b/x86/access.c
@@ -2,6 +2,7 @@
 #include "libcflat.h"
 #include "desc.h"
 #include "processor.h"
+#include "asm/page.h"
 
 #define smp_id() 0
 
@@ -15,9 +16,6 @@ static int cpuid_7_ebx;
 static int cpuid_7_ecx;
 static int invalid_mask;
 
-#define PAGE_SIZE ((pt_element_t)4096)
-#define PAGE_MASK (~(PAGE_SIZE-1))
-
 #define PT_BASE_ADDR_MASK ((pt_element_t)((((pt_element_t)1 << 40) - 1) & PAGE_MASK))
 #define PT_PSE_BASE_ADDR_MASK (PT_BASE_ADDR_MASK & ~(1ull << 21))
 
diff --git a/x86/vmx.h b/x86/vmx.h
index d6bc217cb1cf..27f345190bee 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -4,6 +4,7 @@
 #include "libcflat.h"
 #include "processor.h"
 #include "bitops.h"
+#include "asm/page.h"
 
 struct vmcs {
 	u32 revision_id; /* vmcs revision identifier */
@@ -476,7 +477,6 @@ enum Ctrl1 {
 #define EPT_PGDIR_ENTRIES	(1 << EPT_PGDIR_WIDTH)
 #define EPT_LEVEL_SHIFT(level)	(((level)-1) * EPT_PGDIR_WIDTH + 12)
 #define EPT_ADDR_MASK		GENMASK_ULL(51, 12)
-#define PAGE_MASK		(~(PAGE_SIZE-1))
 #define PAGE_MASK_2M		(~(PAGE_SIZE_2M-1))
 
 #define EPT_VLT_RD		1
-- 
1.8.3.1

Re: [kvm-unit-tests PATCH v8 08/12] x86: Cleanup PAGE_* constants

[Paolo Bonzini]

On 06/06/2016 12:25, Alexander Gordeev wrote:
> Cc: Andrew Jones <drjones@redhat.com>
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> Reviewed-by: Andrew Jones <drjones@redhat.com>
> Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
> ---
>  lib/x86/asm/page.h | 10 +++++++++-
>  x86/access.c       |  4 +---
>  x86/vmx.h          |  2 +-
>  3 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/x86/asm/page.h b/lib/x86/asm/page.h
> index d4580b40f5e8..916462d758ac 100644
> --- a/lib/x86/asm/page.h
> +++ b/lib/x86/asm/page.h
> @@ -7,7 +7,14 @@
>   */
>  
>  
> -#define PAGE_SIZE	4096ul
> +#include <linux/const.h>

No linux/ includes here, but you can just use the existing

#define PAGE_SIZE	4096ul

definition.

Paolo

> +
> +#define PAGE_SHIFT	12
> +#define PAGE_SIZE	(_AC(1,UL) << PAGE_SHIFT)
> +#define PAGE_MASK	(~(PAGE_SIZE-1))
> +
> +#ifndef __ASSEMBLY__
> +
>  #ifdef __x86_64__
>  #define LARGE_PAGE_SIZE	(512 * PAGE_SIZE)
>  #else
> @@ -30,4 +37,5 @@
>  #define	PGDIR_MASK	1023
>  #endif
>  
> +#endif /* !__ASSEMBLY__ */
>  #endif
> diff --git a/x86/access.c b/x86/access.c
> index 4acaee5c6b4d..aaf805d1ea7c 100644
> --- a/x86/access.c
> +++ b/x86/access.c
> @@ -2,6 +2,7 @@
>  #include "libcflat.h"
>  #include "desc.h"
>  #include "processor.h"
> +#include "asm/page.h"
>  
>  #define smp_id() 0
>  
> @@ -15,9 +16,6 @@ static int cpuid_7_ebx;
>  static int cpuid_7_ecx;
>  static int invalid_mask;
>  
> -#define PAGE_SIZE ((pt_element_t)4096)
> -#define PAGE_MASK (~(PAGE_SIZE-1))
> -
>  #define PT_BASE_ADDR_MASK ((pt_element_t)((((pt_element_t)1 << 40) - 1) & PAGE_MASK))
>  #define PT_PSE_BASE_ADDR_MASK (PT_BASE_ADDR_MASK & ~(1ull << 21))
>  
> diff --git a/x86/vmx.h b/x86/vmx.h
> index d6bc217cb1cf..27f345190bee 100644
> --- a/x86/vmx.h
> +++ b/x86/vmx.h
> @@ -4,6 +4,7 @@
>  #include "libcflat.h"
>  #include "processor.h"
>  #include "bitops.h"
> +#include "asm/page.h"
>  
>  struct vmcs {
>  	u32 revision_id; /* vmcs revision identifier */
> @@ -476,7 +477,6 @@ enum Ctrl1 {
>  #define EPT_PGDIR_ENTRIES	(1 << EPT_PGDIR_WIDTH)
>  #define EPT_LEVEL_SHIFT(level)	(((level)-1) * EPT_PGDIR_WIDTH + 12)
>  #define EPT_ADDR_MASK		GENMASK_ULL(51, 12)
> -#define PAGE_MASK		(~(PAGE_SIZE-1))
>  #define PAGE_MASK_2M		(~(PAGE_SIZE_2M-1))
>  
>  #define EPT_VLT_RD		1
> 

Re: [kvm-unit-tests PATCH v8 08/12] x86: Cleanup PAGE_* constants

[Alexander Gordeev]

On Mon, Jun 06, 2016 at 05:21:23PM +0200, Paolo Bonzini wrote:
> > diff --git a/lib/x86/asm/page.h b/lib/x86/asm/page.h
> > index d4580b40f5e8..916462d758ac 100644
> > --- a/lib/x86/asm/page.h
> > +++ b/lib/x86/asm/page.h
> > @@ -7,7 +7,14 @@
> >   */
> >  
> >  
> > -#define PAGE_SIZE	4096ul
> > +#include <linux/const.h>
> 
> No linux/ includes here, but you can just use the existing
> 
> #define PAGE_SIZE	4096ul
> 
> definition.

We have page constants defined this way for every other arch:

lib/arm/asm/page.h
lib/arm64/asm/page.h
lib/asm-generic/page.h (covers ppc64)

Why not to bring x86 in line also?

Thanks!

> Paolo
> 
> > +
> > +#define PAGE_SHIFT	12
> > +#define PAGE_SIZE	(_AC(1,UL) << PAGE_SHIFT)
> > +#define PAGE_MASK	(~(PAGE_SIZE-1))
> > +
> > +#ifndef __ASSEMBLY__
> > +
> >  #ifdef __x86_64__
> >  #define LARGE_PAGE_SIZE	(512 * PAGE_SIZE)
> >  #else
> > @@ -30,4 +37,5 @@
> >  #define	PGDIR_MASK	1023
> >  #endif
> >  
> > +#endif /* !__ASSEMBLY__ */
> >  #endif

Re: [kvm-unit-tests PATCH v8 08/12] x86: Cleanup PAGE_* constants

[Paolo Bonzini]

> We have page constants defined this way for every other arch:
> 
> lib/arm/asm/page.h
> lib/arm64/asm/page.h
> lib/asm-generic/page.h (covers ppc64)
> 
> Why not to bring x86 in line also?

The bug is in the others. :)  There's no reason for kvm-unit-tests to
depend on Linux, and PAGE_SIZE is never used in assembly either.

Paolo

Re: [kvm-unit-tests PATCH v8 08/12] x86: Cleanup PAGE_* constants

[Andrew Jones]

On Wed, Jun 08, 2016 at 03:58:07AM -0400, Paolo Bonzini wrote:
> 
> > We have page constants defined this way for every other arch:
> > 
> > lib/arm/asm/page.h
> > lib/arm64/asm/page.h
> > lib/asm-generic/page.h (covers ppc64)
> > 
> > Why not to bring x86 in line also?
> 
> The bug is in the others. :)  There's no reason for kvm-unit-tests to
> depend on Linux, and PAGE_SIZE is never used in assembly either.

kvm-unit-tests isn't depending on linux when including <linux/...>.
We have our own copies of a few Linux uapi headers in there, like
const.h.

We may not use PAGE_SIZE in assembly yet, but it's certainly a
reasonable thing to expect to do some day. Look at the linux
source

 $ git grep PAGE_SIZE | grep '\.S:' | wc -l
 459

drew

Re: [kvm-unit-tests PATCH v8 08/12] x86: Cleanup PAGE_* constants

[Paolo Bonzini]

----- Original Message -----
> From: "Andrew Jones" <drjones@redhat.com>
> To: "Paolo Bonzini" <pbonzini@redhat.com>
> Cc: "Alexander Gordeev" <agordeev@redhat.com>, kvm@vger.kernel.org, "Thomas Huth" <thuth@redhat.com>, "Radim Krčmář"
> <rkrcmar@redhat.com>
> Sent: Wednesday, June 8, 2016 12:07:41 PM
> Subject: Re: [kvm-unit-tests PATCH v8 08/12] x86: Cleanup PAGE_* constants
> 
> On Wed, Jun 08, 2016 at 03:58:07AM -0400, Paolo Bonzini wrote:
> > 
> > > We have page constants defined this way for every other arch:
> > > 
> > > lib/arm/asm/page.h
> > > lib/arm64/asm/page.h
> > > lib/asm-generic/page.h (covers ppc64)
> > > 
> > > Why not to bring x86 in line also?
> > 
> > The bug is in the others. :)  There's no reason for kvm-unit-tests to
> > depend on Linux, and PAGE_SIZE is never used in assembly either.
> 
> kvm-unit-tests isn't depending on linux when including <linux/...>.
> We have our own copies of a few Linux uapi headers in there, like
> const.h.

Doh, brain fart then, the patch is good.

Paolo

[kvm-unit-tests PATCH v8 09/12] x86: Cleanup PT_*_MASK flags

[Alexander Gordeev]

Cc: Andrew Jones <drjones@redhat.com>
Cc: Thomas Huth <thuth@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
 lib/x86/asm/page.h | 14 +++++++++-----
 lib/x86/vm.c       | 20 ++++++++++----------
 x86/access.c       | 14 +++-----------
 x86/asyncpf.c      |  2 +-
 x86/eventinj.c     | 10 +++++-----
 x86/hypercall.c    |  2 +-
 x86/pku.c          |  2 +-
 x86/rmap_chain.c   |  2 +-
 x86/smap.c         |  6 +++---
 x86/taskswitch2.c  |  2 +-
 10 files changed, 35 insertions(+), 39 deletions(-)

diff --git a/lib/x86/asm/page.h b/lib/x86/asm/page.h
index 916462d758ac..5044a49ab0cc 100644
--- a/lib/x86/asm/page.h
+++ b/lib/x86/asm/page.h
@@ -8,6 +8,7 @@
 
 
 #include <linux/const.h>
+#include <bitops.h>
 
 #define PAGE_SHIFT	12
 #define PAGE_SIZE	(_AC(1,UL) << PAGE_SHIFT)
@@ -21,11 +22,14 @@
 #define LARGE_PAGE_SIZE	(1024 * PAGE_SIZE)
 #endif
 
-#define PTE_PRESENT	(1ull << 0)
-#define PTE_WRITE	(1ull << 1)
-#define PTE_USER	(1ull << 2)
-#define PTE_PSE		(1ull << 7)
-#define PTE_ADDR	(0xffffffffff000ull)
+#define PT_PRESENT_MASK		(1ull << 0)
+#define PT_WRITABLE_MASK	(1ull << 1)
+#define PT_USER_MASK		(1ull << 2)
+#define PT_ACCESSED_MASK	(1ull << 5)
+#define PT_DIRTY_MASK		(1ull << 6)
+#define PT_PAGE_SIZE_MASK	(1ull << 7)
+#define PT64_NX_MASK		(1ull << 63)
+#define PT_ADDR_MASK		GENMASK_ULL(51, 12)
 
 #ifdef __x86_64__
 #define	PAGE_LEVEL	4
diff --git a/lib/x86/vm.c b/lib/x86/vm.c
index 9c94ca5b7fc4..906fbf264291 100644
--- a/lib/x86/vm.c
+++ b/lib/x86/vm.c
@@ -49,16 +49,16 @@ unsigned long *install_pte(unsigned long *cr3,
 
     for (level = PAGE_LEVEL; level > pte_level; --level) {
 	offset = ((unsigned long)virt >> ((level-1) * PGDIR_WIDTH + 12)) & PGDIR_MASK;
-	if (!(pt[offset] & PTE_PRESENT)) {
+	if (!(pt[offset] & PT_PRESENT_MASK)) {
 	    unsigned long *new_pt = pt_page;
             if (!new_pt)
                 new_pt = alloc_page();
             else
                 pt_page = 0;
 	    memset(new_pt, 0, PAGE_SIZE);
-	    pt[offset] = virt_to_phys(new_pt) | PTE_PRESENT | PTE_WRITE | PTE_USER;
+	    pt[offset] = virt_to_phys(new_pt) | PT_PRESENT_MASK | PT_WRITABLE_MASK | PT_USER_MASK;
 	}
-	pt = phys_to_virt(pt[offset] & 0xffffffffff000ull);
+	pt = phys_to_virt(pt[offset] & PT_ADDR_MASK);
     }
     offset = ((unsigned long)virt >> ((level-1) * PGDIR_WIDTH + 12)) & PGDIR_MASK;
     pt[offset] = pte;
@@ -74,11 +74,11 @@ unsigned long *get_pte(unsigned long *cr3, void *virt)
     for (level = PAGE_LEVEL; level > 1; --level) {
 	offset = ((unsigned long)virt >> (((level-1) * PGDIR_WIDTH) + 12)) & PGDIR_MASK;
 	pte = pt[offset];
-	if (!(pte & PTE_PRESENT))
+	if (!(pte & PT_PRESENT_MASK))
 	    return NULL;
-	if (level == 2 && (pte & PTE_PSE))
+	if (level == 2 && (pte & PT_PAGE_SIZE_MASK))
 	    return &pt[offset];
-	pt = phys_to_virt(pte & 0xffffffffff000ull);
+	pt = phys_to_virt(pte & PT_ADDR_MASK);
     }
     offset = ((unsigned long)virt >> (((level-1) * PGDIR_WIDTH) + 12)) & PGDIR_MASK;
     return &pt[offset];
@@ -89,14 +89,14 @@ unsigned long *install_large_page(unsigned long *cr3,
 				  void *virt)
 {
     return install_pte(cr3, 2, virt,
-		       phys | PTE_PRESENT | PTE_WRITE | PTE_USER | PTE_PSE, 0);
+		       phys | PT_PRESENT_MASK | PT_WRITABLE_MASK | PT_USER_MASK | PT_PAGE_SIZE_MASK, 0);
 }
 
 unsigned long *install_page(unsigned long *cr3,
 			    unsigned long phys,
 			    void *virt)
 {
-    return install_pte(cr3, 1, virt, phys | PTE_PRESENT | PTE_WRITE | PTE_USER, 0);
+    return install_pte(cr3, 1, virt, phys | PT_PRESENT_MASK | PT_WRITABLE_MASK | PT_USER_MASK, 0);
 }
 
 
@@ -178,7 +178,7 @@ void *vmalloc(unsigned long size)
 
 uint64_t virt_to_phys_cr3(void *mem)
 {
-    return (*get_pte(phys_to_virt(read_cr3()), mem) & PTE_ADDR) + ((ulong)mem & (PAGE_SIZE - 1));
+    return (*get_pte(phys_to_virt(read_cr3()), mem) & PT_ADDR_MASK) + ((ulong)mem & (PAGE_SIZE - 1));
 }
 
 void vfree(void *mem)
@@ -186,7 +186,7 @@ void vfree(void *mem)
     unsigned long size = ((unsigned long *)mem)[-1];
 
     while (size) {
-	free_page(phys_to_virt(*get_pte(phys_to_virt(read_cr3()), mem) & PTE_ADDR));
+	free_page(phys_to_virt(*get_pte(phys_to_virt(read_cr3()), mem) & PT_ADDR_MASK));
 	mem += PAGE_SIZE;
 	size -= PAGE_SIZE;
     }
diff --git a/x86/access.c b/x86/access.c
index aaf805d1ea7c..0546dbb9ee3a 100644
--- a/x86/access.c
+++ b/x86/access.c
@@ -19,14 +19,6 @@ static int invalid_mask;
 #define PT_BASE_ADDR_MASK ((pt_element_t)((((pt_element_t)1 << 40) - 1) & PAGE_MASK))
 #define PT_PSE_BASE_ADDR_MASK (PT_BASE_ADDR_MASK & ~(1ull << 21))
 
-#define PT_PRESENT_MASK    ((pt_element_t)1 << 0)
-#define PT_WRITABLE_MASK   ((pt_element_t)1 << 1)
-#define PT_USER_MASK       ((pt_element_t)1 << 2)
-#define PT_ACCESSED_MASK   ((pt_element_t)1 << 5)
-#define PT_DIRTY_MASK      ((pt_element_t)1 << 6)
-#define PT_PSE_MASK        ((pt_element_t)1 << 7)
-#define PT_NX_MASK         ((pt_element_t)1 << 63)
-
 #define CR0_WP_MASK (1UL << 16)
 #define CR4_SMEP_MASK (1UL << 20)
 
@@ -493,7 +485,7 @@ void __ac_setup_specific_pages(ac_test_t *at, ac_pool_t *pool, u64 pd_page,
                     pte |= 2ull << 59;
 	    } else {
 		pte = at->phys & PT_PSE_BASE_ADDR_MASK;
-		pte |= PT_PSE_MASK;
+		pte |= PT_PAGE_SIZE_MASK;
                 if (F(AC_PKU_PKEY))
                     pte |= 1ull << 59;
 	    }
@@ -508,7 +500,7 @@ void __ac_setup_specific_pages(ac_test_t *at, ac_pool_t *pool, u64 pd_page,
 	    if (F(AC_PDE_DIRTY))
 		pte |= PT_DIRTY_MASK;
 	    if (F(AC_PDE_NX))
-		pte |= PT_NX_MASK;
+		pte |= PT64_NX_MASK;
 	    if (F(AC_PDE_BIT51))
 		pte |= 1ull << 51;
 	    if (F(AC_PDE_BIT13))
@@ -530,7 +522,7 @@ void __ac_setup_specific_pages(ac_test_t *at, ac_pool_t *pool, u64 pd_page,
 	    if (F(AC_PTE_DIRTY))
 		pte |= PT_DIRTY_MASK;
 	    if (F(AC_PTE_NX))
-		pte |= PT_NX_MASK;
+		pte |= PT64_NX_MASK;
 	    if (F(AC_PTE_BIT51))
 		pte |= 1ull << 51;
 	    at->ptep = &vroot[index];
diff --git a/x86/asyncpf.c b/x86/asyncpf.c
index 304d015888cf..a6051ab03eed 100644
--- a/x86/asyncpf.c
+++ b/x86/asyncpf.c
@@ -69,7 +69,7 @@ static void pf_isr(struct ex_regs *r)
 			report("Got present #PF token %x\n", true, read_cr2());
 			if ((uint32_t)read_cr2() == ~0)
 				break;
-			install_pte(phys_to_virt(read_cr3()), 1, virt, phys | PTE_PRESENT | PTE_WRITE, 0);
+			install_pte(phys_to_virt(read_cr3()), 1, virt, phys | PT_PRESENT_MASK | PT_WRITABLE_MASK, 0);
 			write_cr3(read_cr3());
 			phys = 0;
 			break;
diff --git a/x86/eventinj.c b/x86/eventinj.c
index 84dfe71d8e2a..9ee557b85494 100644
--- a/x86/eventinj.c
+++ b/x86/eventinj.c
@@ -52,7 +52,7 @@ void do_pf_tss(void)
 {
 	printf("PF running\n");
 	install_pte(phys_to_virt(read_cr3()), 1, stack_va,
-		    stack_phys | PTE_PRESENT | PTE_WRITE, 0);
+		    stack_phys | PT_PRESENT_MASK | PT_WRITABLE_MASK, 0);
 	invlpg(stack_va);
 }
 
@@ -363,7 +363,7 @@ int main()
 	printf("Try to divide by 0\n");
 	/* install read only pte */
 	install_pte(phys_to_virt(read_cr3()), 1, stack_va,
-		    stack_phys | PTE_PRESENT, 0);
+		    stack_phys | PT_PRESENT_MASK, 0);
 	invlpg(stack_va);
 	flush_phys_addr(stack_phys);
 	switch_stack(stack_va + 4095);
@@ -382,7 +382,7 @@ int main()
 	set_idt_sel(33, NP_SEL);
 	/* install read only pte */
 	install_pte(phys_to_virt(read_cr3()), 1, stack_va,
-		    stack_phys | PTE_PRESENT, 0);
+		    stack_phys | PT_PRESENT_MASK, 0);
 	invlpg(stack_va);
 	flush_idt_page();
 	flush_phys_addr(stack_phys);
@@ -398,8 +398,8 @@ int main()
 	/* use shadowed stack during interrupt delivery */
 	for (i = 0; i < 4096/sizeof(ulong); i++) {
 		if (!cr3[i]) {
-			cr3[i] = virt_to_phys(pt) | PTE_PRESENT | PTE_WRITE;
-			pt[0] = virt_to_phys(pt) | PTE_PRESENT | PTE_WRITE;
+			cr3[i] = virt_to_phys(pt) | PT_PRESENT_MASK | PT_WRITABLE_MASK;
+			pt[0] = virt_to_phys(pt) | PT_PRESENT_MASK | PT_WRITABLE_MASK;
 #ifndef __x86_64__
 			((ulong*)(i<<22))[1] = 0;
 #else
diff --git a/x86/hypercall.c b/x86/hypercall.c
index 75179a10fc3d..9380f785761a 100644
--- a/x86/hypercall.c
+++ b/x86/hypercall.c
@@ -65,7 +65,7 @@ int main(int ac, char **av)
 	u8 *topmost = (void *) ((1ul << 47) - PAGE_SIZE);
 
 	install_pte(phys_to_virt(read_cr3()), 1, topmost,
-		    virt_to_phys(data1) | PTE_PRESENT | PTE_WRITE, 0);
+		    virt_to_phys(data1) | PT_PRESENT_MASK | PT_WRITABLE_MASK, 0);
 	memset(topmost, 0xcc, PAGE_SIZE);
 	topmost[4093] = 0x0f;
 	topmost[4094] = 0x01;
diff --git a/x86/pku.c b/x86/pku.c
index 488cce3c6fdf..0e7b03fbe60a 100644
--- a/x86/pku.c
+++ b/x86/pku.c
@@ -77,7 +77,7 @@ int main(int ac, char **av)
     wrmsr(MSR_EFER, rdmsr(MSR_EFER) | EFER_LMA);
 
     for (i = 0; i < USER_BASE; i += PAGE_SIZE) {
-        *get_pte(phys_to_virt(read_cr3()), phys_to_virt(i)) &= ~PTE_USER;
+        *get_pte(phys_to_virt(read_cr3()), phys_to_virt(i)) &= ~PT_USER_MASK;
         *get_pte(phys_to_virt(read_cr3()), phys_to_virt(i)) |= ((unsigned long)pkey << PTE_PKEY_BIT);
         invlpg((void *)i);
     }
diff --git a/x86/rmap_chain.c b/x86/rmap_chain.c
index b356a37c358e..7bf6275cab22 100644
--- a/x86/rmap_chain.c
+++ b/x86/rmap_chain.c
@@ -36,7 +36,7 @@ int main (void)
 
     virt_addr += PAGE_SIZE;
     install_pte(phys_to_virt(read_cr3()), 1, virt_addr,
-                0 | PTE_PRESENT | PTE_WRITE, target_page);
+                0 | PT_PRESENT_MASK | PT_WRITABLE_MASK, target_page);
 
     *(unsigned long *)virt_addr = 0;
     printf("PASS\n");
diff --git a/x86/smap.c b/x86/smap.c
index 69dd80915c4a..f316c146542b 100644
--- a/x86/smap.c
+++ b/x86/smap.c
@@ -62,7 +62,7 @@ static void check_smap_nowp(void)
 {
 	test = 0x99;
 
-	*get_pte(phys_to_virt(read_cr3()), USER_ADDR(test)) &= ~PTE_WRITE;
+	*get_pte(phys_to_virt(read_cr3()), USER_ADDR(test)) &= ~PT_WRITABLE_MASK;
 
 	write_cr4(read_cr4() & ~X86_CR4_SMAP);
 	write_cr0(read_cr0() & ~X86_CR0_WP);
@@ -81,7 +81,7 @@ static void check_smap_nowp(void)
 	report("read from user page with SMAP=1, AC=0, WP=0, PTE.U=1 && PTE.W=0", pf_count == 1 && save == 0x99);
 
 	/* Undo changes */
-	*get_pte(phys_to_virt(read_cr3()), USER_ADDR(test)) |= PTE_WRITE;
+	*get_pte(phys_to_virt(read_cr3()), USER_ADDR(test)) |= PT_WRITABLE_MASK;
 
 	write_cr0(read_cr0() | X86_CR0_WP);
 	write_cr3(read_cr3());
@@ -102,7 +102,7 @@ int main(int ac, char **av)
 
 	// Map first 16MB as supervisor pages
 	for (i = 0; i < USER_BASE; i += PAGE_SIZE) {
-		*get_pte(phys_to_virt(read_cr3()), phys_to_virt(i)) &= ~PTE_USER;
+		*get_pte(phys_to_virt(read_cr3()), phys_to_virt(i)) &= ~PT_USER_MASK;
 		invlpg((void *)i);
 	}
 
diff --git a/x86/taskswitch2.c b/x86/taskswitch2.c
index d70b901a08b6..bb7345b2f525 100644
--- a/x86/taskswitch2.c
+++ b/x86/taskswitch2.c
@@ -67,7 +67,7 @@ void do_pf_tss(ulong *error_code)
 	if (*error_code == 0x2) /* write access, not present */
 		test_count++;
 	install_pte(phys_to_virt(read_cr3()), 1, fault_addr,
-		    fault_phys | PTE_PRESENT | PTE_WRITE, 0);
+		    fault_phys | PT_PRESENT_MASK | PT_WRITABLE_MASK, 0);
 }
 
 extern void pf_tss(void);
-- 
1.8.3.1

[kvm-unit-tests PATCH v8 10/12] io: Make ioremap() prototype conform to Linux one

[Alexander Gordeev]

This update also adds missing __iomem specificator which
has been used by existing IO accessors.

Cc: Andrew Jones <drjones@redhat.com>
Cc: Thomas Huth <thuth@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
 lib/asm-generic/io.h | 4 ++--
 lib/ppc64/asm/io.h   | 2 ++
 lib/x86/asm/io.h     | 2 ++
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/lib/asm-generic/io.h b/lib/asm-generic/io.h
index 1834720db2ac..41756dbf01bb 100644
--- a/lib/asm-generic/io.h
+++ b/lib/asm-generic/io.h
@@ -156,10 +156,10 @@ static inline u64 __bswap64(u64 x)
 	({ wmb(); __raw_writeq(cpu_to_le64(b), addr); })
 
 #ifndef ioremap
-static inline void *ioremap(u64 phys_addr, size_t size __unused)
+static inline void __iomem *ioremap(phys_addr_t phys_addr, size_t size __unused)
 {
 	assert(sizeof(long) == 8 || !(phys_addr >> 32));
-	return (void *)(unsigned long)phys_addr;
+	return (void __iomem *)(unsigned long)phys_addr;
 }
 #endif
 
diff --git a/lib/ppc64/asm/io.h b/lib/ppc64/asm/io.h
index 4f2c31b7778e..2b4dd2beff1d 100644
--- a/lib/ppc64/asm/io.h
+++ b/lib/ppc64/asm/io.h
@@ -9,5 +9,7 @@
 #error Undefined byte order
 #endif
 
+#define __iomem
+
 #include <asm-generic/io.h>
 #endif
diff --git a/lib/x86/asm/io.h b/lib/x86/asm/io.h
index c944df4b07c8..83387b581164 100644
--- a/lib/x86/asm/io.h
+++ b/lib/x86/asm/io.h
@@ -1,6 +1,8 @@
 #ifndef _ASM_X86_IO_H_
 #define _ASM_X86_IO_H_
 
+#define __iomem
+
 static inline unsigned char inb(unsigned short port)
 {
     unsigned char value;
-- 
1.8.3.1

[kvm-unit-tests PATCH v8 11/12] x86: io: Factor out ioremap()

[Alexander Gordeev]

Cc: Andrew Jones <drjones@redhat.com>
Cc: Thomas Huth <thuth@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
 lib/x86/asm/io.h |  2 ++
 lib/x86/io.c     | 16 ++++++++++++++++
 x86/vmexit.c     | 10 ++--------
 3 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/lib/x86/asm/io.h b/lib/x86/asm/io.h
index 83387b581164..2436822162de 100644
--- a/lib/x86/asm/io.h
+++ b/lib/x86/asm/io.h
@@ -49,4 +49,6 @@ static inline void *phys_to_virt(unsigned long phys)
     return (void *)phys;
 }
 
+void __iomem *ioremap(phys_addr_t phys_addr, size_t size);
+
 #endif
diff --git a/lib/x86/io.c b/lib/x86/io.c
index d396d42d0535..cc5ac5855366 100644
--- a/lib/x86/io.c
+++ b/lib/x86/io.c
@@ -1,6 +1,8 @@
 #include "libcflat.h"
+#include "vm.h"
 #include "smp.h"
 #include "asm/io.h"
+#include "asm/page.h"
 #ifndef USE_SERIAL
 #define USE_SERIAL
 #endif
@@ -81,3 +83,17 @@ void exit(int code)
         asm volatile("out %0, %1" : : "a"(code), "d"((short)0xf4));
 #endif
 }
+
+void __iomem *ioremap(phys_addr_t phys_addr, size_t size)
+{
+	phys_addr_t base = phys_addr & PAGE_MASK;
+	phys_addr_t offset = phys_addr - base;
+
+	/*
+	 * The kernel sets PTEs for an ioremap() with page cache disabled,
+	 * but we do not do that right now. It would make sense that I/O
+	 * mappings would be uncached - and may help us find bugs when we
+	 * properly map that way.
+	 */
+	return vmap(phys_addr, size) + offset;
+}
diff --git a/x86/vmexit.c b/x86/vmexit.c
index db7dbd88fe2a..c2e1e496918d 100644
--- a/x86/vmexit.c
+++ b/x86/vmexit.c
@@ -371,8 +371,7 @@ int main(int ac, char **av)
 {
 	struct fadt_descriptor_rev1 *fadt;
 	int i;
-	unsigned long membar = 0, base, offset;
-	void *m;
+	unsigned long membar = 0;
 	pcidevaddr_t pcidev;
 
 	smp_init();
@@ -394,12 +393,7 @@ int main(int ac, char **av)
 			}
 			if (pci_bar_is_memory(pcidev, i)) {
 				membar = pci_bar_addr(pcidev, i);
-				base = membar & ~4095;
-				offset = membar - base;
-				m = alloc_vpages(1);
-				
-				install_page((void *)read_cr3(), base, m);
-				pci_test.memaddr = m + offset;
+				pci_test.memaddr = ioremap(membar, PAGE_SIZE);
 			} else {
 				pci_test.iobar = pci_bar_addr(pcidev, i);
 			}
-- 
1.8.3.1

[kvm-unit-tests PATCH v8 12/12] io: Unify IO accessors across architectures

[Alexander Gordeev]

Cc: Andrew Jones <drjones@redhat.com>
Cc: Thomas Huth <thuth@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
 lib/asm-generic/barrier.h | 21 +++++++++++++++++++
 lib/asm-generic/io.h      | 53 ++++++++++++++++++++++++++++++++++++++---------
 lib/ppc64/asm/barrier.h   |  4 ++++
 lib/x86/asm/io.h          | 35 ++++++++++++++++++++-----------
 4 files changed, 91 insertions(+), 22 deletions(-)
 create mode 100644 lib/asm-generic/barrier.h
 create mode 100644 lib/ppc64/asm/barrier.h

diff --git a/lib/asm-generic/barrier.h b/lib/asm-generic/barrier.h
new file mode 100644
index 000000000000..21c88e9184c5
--- /dev/null
+++ b/lib/asm-generic/barrier.h
@@ -0,0 +1,21 @@
+#ifndef _ASM_BARRIER_H_
+#define _ASM_BARRIER_H_
+/*
+ * asm-generic/barrier.h
+ *
+ * Copyright (C) 2016, Red Hat Inc, Alexander Gordeev <agordeev@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.
+ */
+
+#ifndef mb
+#define mb()	asm volatile("":::"memory")
+#endif
+#ifndef rmb
+#define rmb()	asm volatile("":::"memory")
+#endif
+#ifndef wmb
+#define wmb()	asm volatile("":::"memory")
+#endif
+
+#endif /* _ASM_BARRIER_H_ */
diff --git a/lib/asm-generic/io.h b/lib/asm-generic/io.h
index 41756dbf01bb..91a2d7995d3d 100644
--- a/lib/asm-generic/io.h
+++ b/lib/asm-generic/io.h
@@ -11,6 +11,7 @@
  */
 #include "libcflat.h"
 #include "asm/page.h"
+#include "asm/barrier.h"
 
 #ifndef __raw_readb
 static inline u8 __raw_readb(const volatile void *addr)
@@ -127,16 +128,6 @@ static inline u64 __bswap64(u64 x)
 	({ u64 __r = !__cpu_is_be() ? __bswap64(x) : ((u64)x); __r; })
 #define cpu_to_be64 be64_to_cpu
 
-#ifndef mb
-#define mb()	asm volatile("":::"memory")
-#endif
-#ifndef rmb
-#define rmb()	asm volatile("":::"memory")
-#endif
-#ifndef wmb
-#define wmb()	asm volatile("":::"memory")
-#endif
-
 #define readb(addr) \
 	({ u8 __r = __raw_readb(addr); rmb(); __r; })
 #define readw(addr) \
@@ -155,6 +146,48 @@ static inline u64 __bswap64(u64 x)
 #define writeq(b, addr) \
 	({ wmb(); __raw_writeq(cpu_to_le64(b), addr); })
 
+#ifndef inb
+static inline uint8_t inb(unsigned long port)
+{
+	return readb((const volatile void __iomem *)port);
+}
+#endif
+
+#ifndef inw
+static inline uint16_t inw(unsigned long port)
+{
+	return readw((const volatile void __iomem *)port);
+}
+#endif
+
+#ifndef inl
+static inline uint32_t inl(unsigned long port)
+{
+	return readl((const volatile void __iomem *)port);
+}
+#endif
+
+#ifndef outb
+static inline void outb(uint8_t value, unsigned long port)
+{
+	writeb(value, (volatile void __iomem *)port);
+}
+#endif
+
+#ifndef outw
+static inline void outw(uint16_t value, unsigned long port)
+{
+	writew(value, (volatile void __iomem *)port);
+}
+#endif
+
+#ifndef outl
+static inline void outl(uint32_t value, unsigned long port)
+{
+	writel(value, (volatile void __iomem *)port);
+}
+#endif
+
 #ifndef ioremap
 static inline void __iomem *ioremap(phys_addr_t phys_addr, size_t size __unused)
 {
diff --git a/lib/ppc64/asm/barrier.h b/lib/ppc64/asm/barrier.h
new file mode 100644
index 000000000000..5c772ded52e2
--- /dev/null
+++ b/lib/ppc64/asm/barrier.h
@@ -0,0 +1,4 @@
+#ifndef _ASMPPC64_BARRIER_H_
+#define _ASMPPC64_BARRIER_H_
+#include <asm-generic/barrier.h>
+#endif
diff --git a/lib/x86/asm/io.h b/lib/x86/asm/io.h
index 2436822162de..35a5c7347411 100644
--- a/lib/x86/asm/io.h
+++ b/lib/x86/asm/io.h
@@ -3,52 +3,63 @@
 
 #define __iomem
 
-static inline unsigned char inb(unsigned short port)
+#define inb inb
+static inline uint8_t inb(unsigned long port)
 {
     unsigned char value;
-    asm volatile("inb %w1, %0" : "=a" (value) : "Nd" (port));
+    asm volatile("inb %w1, %0" : "=a" (value) : "Nd" ((unsigned short)port));
     return value;
 }
 
-static inline unsigned short inw(unsigned short port)
+#define inw inw
+static inline uint16_t inw(unsigned long port)
 {
     unsigned short value;
-    asm volatile("inw %w1, %0" : "=a" (value) : "Nd" (port));
+    asm volatile("inw %w1, %0" : "=a" (value) : "Nd" ((unsigned short)port));
     return value;
 }
 
-static inline unsigned int inl(unsigned short port)
+#define inl inl
+static inline uint32_t inl(unsigned long port)
 {
     unsigned int value;
-    asm volatile("inl %w1, %0" : "=a" (value) : "Nd" (port));
+    asm volatile("inl %w1, %0" : "=a" (value) : "Nd" ((unsigned short)port));
     return value;
 }
 
-static inline void outb(unsigned char value, unsigned short port)
+#define outb outb
+static inline void outb(uint8_t value, unsigned long port)
 {
-    asm volatile("outb %b0, %w1" : : "a"(value), "Nd"(port));
+    asm volatile("outb %b0, %w1" : : "a"(value), "Nd"((unsigned short)port));
 }
 
-static inline void outw(unsigned short value, unsigned short port)
+#define outw outw
+static inline void outw(uint16_t value, unsigned long port)
 {
-    asm volatile("outw %w0, %w1" : : "a"(value), "Nd"(port));
+    asm volatile("outw %w0, %w1" : : "a"(value), "Nd"((unsigned short)port));
 }
 
-static inline void outl(unsigned int value, unsigned short port)
+#define outl outl
+static inline void outl(uint32_t value, unsigned long port)
 {
-    asm volatile("outl %0, %w1" : : "a"(value), "Nd"(port));
+    asm volatile("outl %0, %w1" : : "a"(value), "Nd"((unsigned short)port));
 }
 
+#define virt_to_phys virt_to_phys
 static inline unsigned long virt_to_phys(const void *virt)
 {
     return (unsigned long)virt;
 }
 
+#define phys_to_virt phys_to_virt
 static inline void *phys_to_virt(unsigned long phys)
 {
     return (void *)phys;
 }
 
+#define ioremap ioremap
 void __iomem *ioremap(phys_addr_t phys_addr, size_t size);
 
+#include <asm-generic/io.h>
+
 #endif
-- 
1.8.3.1

Re: [kvm-unit-tests PATCH v8 12/12] io: Unify IO accessors across architectures

[Andrew Jones]

On Mon, Jun 06, 2016 at 12:25:29PM +0200, Alexander Gordeev wrote:
> Cc: Andrew Jones <drjones@redhat.com>
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
> ---
>  lib/asm-generic/barrier.h | 21 +++++++++++++++++++
>  lib/asm-generic/io.h      | 53 ++++++++++++++++++++++++++++++++++++++---------
>  lib/ppc64/asm/barrier.h   |  4 ++++
>  lib/x86/asm/io.h          | 35 ++++++++++++++++++++-----------
>  4 files changed, 91 insertions(+), 22 deletions(-)
>  create mode 100644 lib/asm-generic/barrier.h
>  create mode 100644 lib/ppc64/asm/barrier.h
> 
> diff --git a/lib/asm-generic/barrier.h b/lib/asm-generic/barrier.h
> new file mode 100644
> index 000000000000..21c88e9184c5
> --- /dev/null
> +++ b/lib/asm-generic/barrier.h
> @@ -0,0 +1,21 @@
> +#ifndef _ASM_BARRIER_H_
> +#define _ASM_BARRIER_H_
> +/*
> + * asm-generic/barrier.h
> + *
> + * Copyright (C) 2016, Red Hat Inc, Alexander Gordeev <agordeev@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.
> + */
> +
> +#ifndef mb
> +#define mb()	asm volatile("":::"memory")
> +#endif
> +#ifndef rmb
> +#define rmb()	asm volatile("":::"memory")
> +#endif
> +#ifndef wmb
> +#define wmb()	asm volatile("":::"memory")
> +#endif
> +
> +#endif /* _ASM_BARRIER_H_ */
> diff --git a/lib/asm-generic/io.h b/lib/asm-generic/io.h
> index 41756dbf01bb..91a2d7995d3d 100644
> --- a/lib/asm-generic/io.h
> +++ b/lib/asm-generic/io.h
> @@ -11,6 +11,7 @@
>   */
>  #include "libcflat.h"
>  #include "asm/page.h"
> +#include "asm/barrier.h"
>  
>  #ifndef __raw_readb
>  static inline u8 __raw_readb(const volatile void *addr)
> @@ -127,16 +128,6 @@ static inline u64 __bswap64(u64 x)
>  	({ u64 __r = !__cpu_is_be() ? __bswap64(x) : ((u64)x); __r; })
>  #define cpu_to_be64 be64_to_cpu
>  
> -#ifndef mb
> -#define mb()	asm volatile("":::"memory")
> -#endif
> -#ifndef rmb
> -#define rmb()	asm volatile("":::"memory")
> -#endif
> -#ifndef wmb
> -#define wmb()	asm volatile("":::"memory")
> -#endif


The movement of the barriers seems unrelated. Or was there
a need to do so for this patch that I'm missing? If it is
unrelated then it should be a separate patch.

> -
>  #define readb(addr) \
>  	({ u8 __r = __raw_readb(addr); rmb(); __r; })
>  #define readw(addr) \
> @@ -155,6 +146,48 @@ static inline u64 __bswap64(u64 x)
>  #define writeq(b, addr) \
>  	({ wmb(); __raw_writeq(cpu_to_le64(b), addr); })
>  
> +#ifndef inb
> +static inline uint8_t inb(unsigned long port)
> +{
> +	return readb((const volatile void __iomem *)port);
> +}
> +#endif
> +
> +#ifndef inw
> +static inline uint16_t inw(unsigned long port)
> +{
> +	return readw((const volatile void __iomem *)port);
> +}
> +#endif
> +
> +#ifndef inl
> +static inline uint32_t inl(unsigned long port)
> +{
> +	return readl((const volatile void __iomem *)port);
> +}
> +#endif
> +
> +#ifndef outb
> +static inline void outb(uint8_t value, unsigned long port)
> +{
> +	writeb(value, (volatile void __iomem *)port);
> +}
> +#endif
> +
> +#ifndef outw
> +static inline void outw(uint16_t value, unsigned long port)
> +{
> +	writew(value, (volatile void __iomem *)port);
> +}
> +#endif
> +
> +#ifndef outl
> +static inline void outl(uint32_t value, unsigned long port)
> +{
> +	writel(value, (volatile void __iomem *)port);
> +}
> +#endif
> +
>  #ifndef ioremap
>  static inline void __iomem *ioremap(phys_addr_t phys_addr, size_t size __unused)
>  {
> diff --git a/lib/ppc64/asm/barrier.h b/lib/ppc64/asm/barrier.h
> new file mode 100644
> index 000000000000..5c772ded52e2
> --- /dev/null
> +++ b/lib/ppc64/asm/barrier.h
> @@ -0,0 +1,4 @@
> +#ifndef _ASMPPC64_BARRIER_H_
> +#define _ASMPPC64_BARRIER_H_
> +#include <asm-generic/barrier.h>
> +#endif
> diff --git a/lib/x86/asm/io.h b/lib/x86/asm/io.h
> index 2436822162de..35a5c7347411 100644
> --- a/lib/x86/asm/io.h
> +++ b/lib/x86/asm/io.h
> @@ -3,52 +3,63 @@
>  
>  #define __iomem
>  
> -static inline unsigned char inb(unsigned short port)
> +#define inb inb
> +static inline uint8_t inb(unsigned long port)
>  {
>      unsigned char value;
> -    asm volatile("inb %w1, %0" : "=a" (value) : "Nd" (port));
> +    asm volatile("inb %w1, %0" : "=a" (value) : "Nd" ((unsigned short)port));
>      return value;
>  }
>  
> -static inline unsigned short inw(unsigned short port)
> +#define inw inw
> +static inline uint16_t inw(unsigned long port)
>  {
>      unsigned short value;
> -    asm volatile("inw %w1, %0" : "=a" (value) : "Nd" (port));
> +    asm volatile("inw %w1, %0" : "=a" (value) : "Nd" ((unsigned short)port));
>      return value;
>  }
>  
> -static inline unsigned int inl(unsigned short port)
> +#define inl inl
> +static inline uint32_t inl(unsigned long port)
>  {
>      unsigned int value;
> -    asm volatile("inl %w1, %0" : "=a" (value) : "Nd" (port));
> +    asm volatile("inl %w1, %0" : "=a" (value) : "Nd" ((unsigned short)port));
>      return value;
>  }
>  
> -static inline void outb(unsigned char value, unsigned short port)
> +#define outb outb
> +static inline void outb(uint8_t value, unsigned long port)
>  {
> -    asm volatile("outb %b0, %w1" : : "a"(value), "Nd"(port));
> +    asm volatile("outb %b0, %w1" : : "a"(value), "Nd"((unsigned short)port));
>  }
>  
> -static inline void outw(unsigned short value, unsigned short port)
> +#define outw outw
> +static inline void outw(uint16_t value, unsigned long port)
>  {
> -    asm volatile("outw %w0, %w1" : : "a"(value), "Nd"(port));
> +    asm volatile("outw %w0, %w1" : : "a"(value), "Nd"((unsigned short)port));
>  }
>  
> -static inline void outl(unsigned int value, unsigned short port)
> +#define outl outl
> +static inline void outl(uint32_t value, unsigned long port)
>  {
> -    asm volatile("outl %0, %w1" : : "a"(value), "Nd"(port));
> +    asm volatile("outl %0, %w1" : : "a"(value), "Nd"((unsigned short)port));
>  }
>  
> +#define virt_to_phys virt_to_phys
>  static inline unsigned long virt_to_phys(const void *virt)
>  {
>      return (unsigned long)virt;
>  }
>  
> +#define phys_to_virt phys_to_virt
>  static inline void *phys_to_virt(unsigned long phys)
>  {
>      return (void *)phys;
>  }
>  
> +#define ioremap ioremap
>  void __iomem *ioremap(phys_addr_t phys_addr, size_t size);
>  
> +#include <asm-generic/io.h>
> +
>  #endif
> -- 
> 1.8.3.1
>

Otherwise looks good.

Thanks,
drew

[kvm-unit-tests PATCH v9 12/12] ppc64: Introduce lib/ppc64/asm/barrier.h

[Alexander Gordeev]

Cc: Andrew Jones <drjones@redhat.com>
Cc: Thomas Huth <thuth@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>

Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
 lib/asm-generic/barrier.h | 21 +++++++++++++++++++++
 lib/asm-generic/io.h      | 10 ----------
 lib/ppc64/asm/barrier.h   |  4 ++++
 3 files changed, 25 insertions(+), 10 deletions(-)
 create mode 100644 lib/asm-generic/barrier.h
 create mode 100644 lib/ppc64/asm/barrier.h

diff --git a/lib/asm-generic/barrier.h b/lib/asm-generic/barrier.h
new file mode 100644
index 000000000000..21c88e9184c5
--- /dev/null
+++ b/lib/asm-generic/barrier.h
@@ -0,0 +1,21 @@
+#ifndef _ASM_BARRIER_H_
+#define _ASM_BARRIER_H_
+/*
+ * asm-generic/barrier.h
+ *
+ * Copyright (C) 2016, Red Hat Inc, Alexander Gordeev <agordeev@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.
+ */
+
+#ifndef mb
+#define mb()	asm volatile("":::"memory")
+#endif
+#ifndef rmb
+#define rmb()	asm volatile("":::"memory")
+#endif
+#ifndef wmb
+#define wmb()	asm volatile("":::"memory")
+#endif
+
+#endif /* _ASM_BARRIER_H_ */
diff --git a/lib/asm-generic/io.h b/lib/asm-generic/io.h
index 41756dbf01bb..842868ab9d58 100644
--- a/lib/asm-generic/io.h
+++ b/lib/asm-generic/io.h
@@ -127,16 +127,6 @@ static inline u64 __bswap64(u64 x)
 	({ u64 __r = !__cpu_is_be() ? __bswap64(x) : ((u64)x); __r; })
 #define cpu_to_be64 be64_to_cpu
 
-#ifndef mb
-#define mb()	asm volatile("":::"memory")
-#endif
-#ifndef rmb
-#define rmb()	asm volatile("":::"memory")
-#endif
-#ifndef wmb
-#define wmb()	asm volatile("":::"memory")
-#endif
-
 #define readb(addr) \
 	({ u8 __r = __raw_readb(addr); rmb(); __r; })
 #define readw(addr) \
diff --git a/lib/ppc64/asm/barrier.h b/lib/ppc64/asm/barrier.h
new file mode 100644
index 000000000000..5c772ded52e2
--- /dev/null
+++ b/lib/ppc64/asm/barrier.h
@@ -0,0 +1,4 @@
+#ifndef _ASMPPC64_BARRIER_H_
+#define _ASMPPC64_BARRIER_H_
+#include <asm-generic/barrier.h>
+#endif
-- 
1.8.3.1

Re: [kvm-unit-tests PATCH v9 12/12] ppc64: Introduce lib/ppc64/asm/barrier.h

[Andrew Jones]

On Fri, Jun 10, 2016 at 08:36:55AM +0200, Alexander Gordeev wrote:
> Cc: Andrew Jones <drjones@redhat.com>
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> 
> Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
> ---
>  lib/asm-generic/barrier.h | 21 +++++++++++++++++++++
>  lib/asm-generic/io.h      | 10 ----------
>  lib/ppc64/asm/barrier.h   |  4 ++++
>  3 files changed, 25 insertions(+), 10 deletions(-)
>  create mode 100644 lib/asm-generic/barrier.h
>  create mode 100644 lib/ppc64/asm/barrier.h
> 
> diff --git a/lib/asm-generic/barrier.h b/lib/asm-generic/barrier.h
> new file mode 100644
> index 000000000000..21c88e9184c5
> --- /dev/null
> +++ b/lib/asm-generic/barrier.h
> @@ -0,0 +1,21 @@
> +#ifndef _ASM_BARRIER_H_
> +#define _ASM_BARRIER_H_
> +/*
> + * asm-generic/barrier.h
> + *
> + * Copyright (C) 2016, Red Hat Inc, Alexander Gordeev <agordeev@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.
> + */
> +
> +#ifndef mb
> +#define mb()	asm volatile("":::"memory")
> +#endif
> +#ifndef rmb
> +#define rmb()	asm volatile("":::"memory")
> +#endif
> +#ifndef wmb
> +#define wmb()	asm volatile("":::"memory")
> +#endif
> +
> +#endif /* _ASM_BARRIER_H_ */
> diff --git a/lib/asm-generic/io.h b/lib/asm-generic/io.h
> index 41756dbf01bb..842868ab9d58 100644
> --- a/lib/asm-generic/io.h
> +++ b/lib/asm-generic/io.h
> @@ -127,16 +127,6 @@ static inline u64 __bswap64(u64 x)
>  	({ u64 __r = !__cpu_is_be() ? __bswap64(x) : ((u64)x); __r; })
>  #define cpu_to_be64 be64_to_cpu
>  
> -#ifndef mb
> -#define mb()	asm volatile("":::"memory")
> -#endif
> -#ifndef rmb
> -#define rmb()	asm volatile("":::"memory")
> -#endif
> -#ifndef wmb
> -#define wmb()	asm volatile("":::"memory")
> -#endif
> -
>  #define readb(addr) \
>  	({ u8 __r = __raw_readb(addr); rmb(); __r; })
>  #define readw(addr) \
> diff --git a/lib/ppc64/asm/barrier.h b/lib/ppc64/asm/barrier.h
> new file mode 100644
> index 000000000000..5c772ded52e2
> --- /dev/null
> +++ b/lib/ppc64/asm/barrier.h
> @@ -0,0 +1,4 @@
> +#ifndef _ASMPPC64_BARRIER_H_
> +#define _ASMPPC64_BARRIER_H_
> +#include <asm-generic/barrier.h>
> +#endif
> -- 
> 1.8.3.1
>

Reviewed-by: Andrew Jones <drjones@redhat.com>

but... comment about this one in the next patch

Re: [kvm-unit-tests PATCH v9 12/12] ppc64: Introduce lib/ppc64/asm/barrier.h

[Andrew Jones]

On Fri, Jun 10, 2016 at 09:01:52AM +0200, Andrew Jones wrote:
> On Fri, Jun 10, 2016 at 08:36:55AM +0200, Alexander Gordeev wrote:
> > Cc: Andrew Jones <drjones@redhat.com>
> > Cc: Thomas Huth <thuth@redhat.com>
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: Radim Krčmář <rkrcmar@redhat.com>
> > 
> > Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
> > ---
> >  lib/asm-generic/barrier.h | 21 +++++++++++++++++++++
> >  lib/asm-generic/io.h      | 10 ----------
> >  lib/ppc64/asm/barrier.h   |  4 ++++
> >  3 files changed, 25 insertions(+), 10 deletions(-)
> >  create mode 100644 lib/asm-generic/barrier.h
> >  create mode 100644 lib/ppc64/asm/barrier.h
> > 
> > diff --git a/lib/asm-generic/barrier.h b/lib/asm-generic/barrier.h
> > new file mode 100644
> > index 000000000000..21c88e9184c5
> > --- /dev/null
> > +++ b/lib/asm-generic/barrier.h
> > @@ -0,0 +1,21 @@
> > +#ifndef _ASM_BARRIER_H_
> > +#define _ASM_BARRIER_H_
> > +/*
> > + * asm-generic/barrier.h
> > + *
> > + * Copyright (C) 2016, Red Hat Inc, Alexander Gordeev <agordeev@redhat.com>
> > + *
> > + * This work is licensed under the terms of the GNU LGPL, version 2.
> > + */
> > +
> > +#ifndef mb
> > +#define mb()	asm volatile("":::"memory")
> > +#endif
> > +#ifndef rmb
> > +#define rmb()	asm volatile("":::"memory")
> > +#endif
> > +#ifndef wmb
> > +#define wmb()	asm volatile("":::"memory")
> > +#endif
> > +
> > +#endif /* _ASM_BARRIER_H_ */
> > diff --git a/lib/asm-generic/io.h b/lib/asm-generic/io.h
> > index 41756dbf01bb..842868ab9d58 100644
> > --- a/lib/asm-generic/io.h
> > +++ b/lib/asm-generic/io.h
> > @@ -127,16 +127,6 @@ static inline u64 __bswap64(u64 x)
> >  	({ u64 __r = !__cpu_is_be() ? __bswap64(x) : ((u64)x); __r; })
> >  #define cpu_to_be64 be64_to_cpu
> >  
> > -#ifndef mb
> > -#define mb()	asm volatile("":::"memory")
> > -#endif
> > -#ifndef rmb
> > -#define rmb()	asm volatile("":::"memory")
> > -#endif
> > -#ifndef wmb
> > -#define wmb()	asm volatile("":::"memory")
> > -#endif
> > -
> >  #define readb(addr) \
> >  	({ u8 __r = __raw_readb(addr); rmb(); __r; })
> >  #define readw(addr) \
> > diff --git a/lib/ppc64/asm/barrier.h b/lib/ppc64/asm/barrier.h
> > new file mode 100644
> > index 000000000000..5c772ded52e2
> > --- /dev/null
> > +++ b/lib/ppc64/asm/barrier.h
> > @@ -0,0 +1,4 @@
> > +#ifndef _ASMPPC64_BARRIER_H_
> > +#define _ASMPPC64_BARRIER_H_
> > +#include <asm-generic/barrier.h>
> > +#endif
> > -- 
> > 1.8.3.1
> >
> 
> Reviewed-by: Andrew Jones <drjones@redhat.com>
> 
> but... comment about this one in the next patch

Actually see that comment in the next patch. I changed my
mind about this one, thus Un-Reviewed-by: drew

[kvm-unit-tests PATCH v10 12/12] ppc64: Introduce lib/ppc64/asm/barrier.h

[Alexander Gordeev]

Cc: Andrew Jones <drjones@redhat.com>
Cc: Thomas Huth <thuth@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>

Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
 lib/asm-generic/barrier.h | 21 +++++++++++++++++++++
 lib/asm-generic/io.h      | 11 +----------
 lib/ppc64/asm/barrier.h   |  4 ++++
 3 files changed, 26 insertions(+), 10 deletions(-)
 create mode 100644 lib/asm-generic/barrier.h
 create mode 100644 lib/ppc64/asm/barrier.h

diff --git a/lib/asm-generic/barrier.h b/lib/asm-generic/barrier.h
new file mode 100644
index 000000000000..21c88e9184c5
--- /dev/null
+++ b/lib/asm-generic/barrier.h
@@ -0,0 +1,21 @@
+#ifndef _ASM_BARRIER_H_
+#define _ASM_BARRIER_H_
+/*
+ * asm-generic/barrier.h
+ *
+ * Copyright (C) 2016, Red Hat Inc, Alexander Gordeev <agordeev@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.
+ */
+
+#ifndef mb
+#define mb()	asm volatile("":::"memory")
+#endif
+#ifndef rmb
+#define rmb()	asm volatile("":::"memory")
+#endif
+#ifndef wmb
+#define wmb()	asm volatile("":::"memory")
+#endif
+
+#endif /* _ASM_BARRIER_H_ */
diff --git a/lib/asm-generic/io.h b/lib/asm-generic/io.h
index 41756dbf01bb..24725c3cd786 100644
--- a/lib/asm-generic/io.h
+++ b/lib/asm-generic/io.h
@@ -11,6 +11,7 @@
  */
 #include "libcflat.h"
 #include "asm/page.h"
+#include "asm/barrier.h"
 
 #ifndef __raw_readb
 static inline u8 __raw_readb(const volatile void *addr)
@@ -127,16 +128,6 @@ static inline u64 __bswap64(u64 x)
 	({ u64 __r = !__cpu_is_be() ? __bswap64(x) : ((u64)x); __r; })
 #define cpu_to_be64 be64_to_cpu
 
-#ifndef mb
-#define mb()	asm volatile("":::"memory")
-#endif
-#ifndef rmb
-#define rmb()	asm volatile("":::"memory")
-#endif
-#ifndef wmb
-#define wmb()	asm volatile("":::"memory")
-#endif
-
 #define readb(addr) \
 	({ u8 __r = __raw_readb(addr); rmb(); __r; })
 #define readw(addr) \
diff --git a/lib/ppc64/asm/barrier.h b/lib/ppc64/asm/barrier.h
new file mode 100644
index 000000000000..5c772ded52e2
--- /dev/null
+++ b/lib/ppc64/asm/barrier.h
@@ -0,0 +1,4 @@
+#ifndef _ASMPPC64_BARRIER_H_
+#define _ASMPPC64_BARRIER_H_
+#include <asm-generic/barrier.h>
+#endif
-- 
1.8.3.1

Re: [kvm-unit-tests PATCH v10 12/12] ppc64: Introduce lib/ppc64/asm/barrier.h

[Andrew Jones]

On Fri, Jun 10, 2016 at 10:17:19AM +0200, Alexander Gordeev wrote:
> Cc: Andrew Jones <drjones@redhat.com>
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> 
> Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
> ---
>  lib/asm-generic/barrier.h | 21 +++++++++++++++++++++
>  lib/asm-generic/io.h      | 11 +----------
>  lib/ppc64/asm/barrier.h   |  4 ++++
>  3 files changed, 26 insertions(+), 10 deletions(-)
>  create mode 100644 lib/asm-generic/barrier.h
>  create mode 100644 lib/ppc64/asm/barrier.h

Reviewed-by: Andrew Jones <drjones@redhat.com>

> 
> diff --git a/lib/asm-generic/barrier.h b/lib/asm-generic/barrier.h
> new file mode 100644
> index 000000000000..21c88e9184c5
> --- /dev/null
> +++ b/lib/asm-generic/barrier.h
> @@ -0,0 +1,21 @@
> +#ifndef _ASM_BARRIER_H_
> +#define _ASM_BARRIER_H_
> +/*
> + * asm-generic/barrier.h
> + *
> + * Copyright (C) 2016, Red Hat Inc, Alexander Gordeev <agordeev@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.
> + */
> +
> +#ifndef mb
> +#define mb()	asm volatile("":::"memory")
> +#endif
> +#ifndef rmb
> +#define rmb()	asm volatile("":::"memory")
> +#endif
> +#ifndef wmb
> +#define wmb()	asm volatile("":::"memory")
> +#endif
> +
> +#endif /* _ASM_BARRIER_H_ */
> diff --git a/lib/asm-generic/io.h b/lib/asm-generic/io.h
> index 41756dbf01bb..24725c3cd786 100644
> --- a/lib/asm-generic/io.h
> +++ b/lib/asm-generic/io.h
> @@ -11,6 +11,7 @@
>   */
>  #include "libcflat.h"
>  #include "asm/page.h"
> +#include "asm/barrier.h"
>  
>  #ifndef __raw_readb
>  static inline u8 __raw_readb(const volatile void *addr)
> @@ -127,16 +128,6 @@ static inline u64 __bswap64(u64 x)
>  	({ u64 __r = !__cpu_is_be() ? __bswap64(x) : ((u64)x); __r; })
>  #define cpu_to_be64 be64_to_cpu
>  
> -#ifndef mb
> -#define mb()	asm volatile("":::"memory")
> -#endif
> -#ifndef rmb
> -#define rmb()	asm volatile("":::"memory")
> -#endif
> -#ifndef wmb
> -#define wmb()	asm volatile("":::"memory")
> -#endif
> -
>  #define readb(addr) \
>  	({ u8 __r = __raw_readb(addr); rmb(); __r; })
>  #define readw(addr) \
> diff --git a/lib/ppc64/asm/barrier.h b/lib/ppc64/asm/barrier.h
> new file mode 100644
> index 000000000000..5c772ded52e2
> --- /dev/null
> +++ b/lib/ppc64/asm/barrier.h
> @@ -0,0 +1,4 @@
> +#ifndef _ASMPPC64_BARRIER_H_
> +#define _ASMPPC64_BARRIER_H_
> +#include <asm-generic/barrier.h>
> +#endif
> -- 
> 1.8.3.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [kvm-unit-tests PATCH v10 12/12] ppc64: Introduce lib/ppc64/asm/barrier.h

[Paolo Bonzini]

On 13/06/2016 12:49, Andrew Jones wrote:
>> +
>> +#ifndef mb
>> +#define mb()	asm volatile("":::"memory")
>> +#endif
>> +#ifndef rmb
>> +#define rmb()	asm volatile("":::"memory")
>> +#endif
>> +#ifndef wmb
>> +#define wmb()	asm volatile("":::"memory")
>> +#endif

This is certainly wrong for PPC64.

Paolo

Re: [kvm-unit-tests PATCH v10 12/12] ppc64: Introduce lib/ppc64/asm/barrier.h

[Andrew Jones]

On Mon, Jun 13, 2016 at 03:34:53PM +0200, Paolo Bonzini wrote:
> 
> 
> On 13/06/2016 12:49, Andrew Jones wrote:
> >> +
> >> +#ifndef mb
> >> +#define mb()	asm volatile("":::"memory")
> >> +#endif
> >> +#ifndef rmb
> >> +#define rmb()	asm volatile("":::"memory")
> >> +#endif
> >> +#ifndef wmb
> >> +#define wmb()	asm volatile("":::"memory")
> >> +#endif
> 
> This is certainly wrong for PPC64.

Using the generic ones is good enough for ppc64 for now,
since it doesn't use them. As soon as it's necessary to have
proper barriers, then lib/ppc64/asm/barrier.h should be
updated to stop including this, and to define their own.

Actually we could drop the ppc64 barrier.h patch for now,
but that relies on the compiler not generating unused code,
otherwise including asm-generic/io.h would break compilation.

Thanks,
drew

Re: [kvm-unit-tests PATCH v10 12/12] ppc64: Introduce lib/ppc64/asm/barrier.h

[Paolo Bonzini]

On 13/06/2016 16:04, Andrew Jones wrote:
> Using the generic ones is good enough for ppc64 for now,
> since it doesn't use them. As soon as it's necessary to have
> proper barriers, then lib/ppc64/asm/barrier.h should be
> updated to stop including this, and to define their own.
> 
> Actually we could drop the ppc64 barrier.h patch for now,
> but that relies on the compiler not generating unused code,
> otherwise including asm-generic/io.h would break compilation.

Currently the sole users of the barriers are in macros, and you can be
pretty sure that the compiler will not complain about unused macros. :)

Paolo

Re: [kvm-unit-tests PATCH v10 12/12] ppc64: Introduce lib/ppc64/asm/barrier.h

[Paolo Bonzini]

On 13/06/2016 16:04, Andrew Jones wrote:
> On Mon, Jun 13, 2016 at 03:34:53PM +0200, Paolo Bonzini wrote:
>>
>>
>> On 13/06/2016 12:49, Andrew Jones wrote:
>>>> +
>>>> +#ifndef mb
>>>> +#define mb()	asm volatile("":::"memory")
>>>> +#endif
>>>> +#ifndef rmb
>>>> +#define rmb()	asm volatile("":::"memory")
>>>> +#endif
>>>> +#ifndef wmb
>>>> +#define wmb()	asm volatile("":::"memory")
>>>> +#endif
>>
>> This is certainly wrong for PPC64.
> 
> Using the generic ones is good enough for ppc64 for now,
> since it doesn't use them. As soon as it's necessary to have
> proper barriers, then lib/ppc64/asm/barrier.h should be
> updated to stop including this, and to define their own.

Let's just do it right:

diff --git a/lib/ppc64/asm/barrier.h b/lib/ppc64/asm/barrier.h
index 5c772de..76f6efa 100644
--- a/lib/ppc64/asm/barrier.h
+++ b/lib/ppc64/asm/barrier.h
@@ -1,4 +1,9 @@
 #ifndef _ASMPPC64_BARRIER_H_
 #define _ASMPPC64_BARRIER_H_
+
+#define mb() asm volatile("sync":::"memory")
+#define rmb() asm volatile("sync":::"memory")
+#define wmb() asm volatile("sync":::"memory")
+
 #include <asm-generic/barrier.h>
 #endif

Paolo

[kvm-unit-tests PATCH v9 13/12] io: Unify IO accessors across architectures

[Alexander Gordeev]

Cc: Andrew Jones <drjones@redhat.com>
Cc: Thomas Huth <thuth@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>

Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
 lib/asm-generic/io.h | 43 +++++++++++++++++++++++++++++++++++++++++++
 lib/x86/asm/io.h     | 35 +++++++++++++++++++++++------------
 2 files changed, 66 insertions(+), 12 deletions(-)

diff --git a/lib/asm-generic/io.h b/lib/asm-generic/io.h
index 842868ab9d58..91a2d7995d3d 100644
--- a/lib/asm-generic/io.h
+++ b/lib/asm-generic/io.h
@@ -11,6 +11,7 @@
  */
 #include "libcflat.h"
 #include "asm/page.h"
+#include "asm/barrier.h"
 
 #ifndef __raw_readb
 static inline u8 __raw_readb(const volatile void *addr)
@@ -145,6 +146,48 @@ static inline u64 __bswap64(u64 x)
 #define writeq(b, addr) \
 	({ wmb(); __raw_writeq(cpu_to_le64(b), addr); })
 
+#ifndef inb
+static inline uint8_t inb(unsigned long port)
+{
+	return readb((const volatile void __iomem *)port);
+}
+#endif
+
+#ifndef inw
+static inline uint16_t inw(unsigned long port)
+{
+	return readw((const volatile void __iomem *)port);
+}
+#endif
+
+#ifndef inl
+static inline uint32_t inl(unsigned long port)
+{
+	return readl((const volatile void __iomem *)port);
+}
+#endif
+
+#ifndef outb
+static inline void outb(uint8_t value, unsigned long port)
+{
+	writeb(value, (volatile void __iomem *)port);
+}
+#endif
+
+#ifndef outw
+static inline void outw(uint16_t value, unsigned long port)
+{
+	writew(value, (volatile void __iomem *)port);
+}
+#endif
+
+#ifndef outl
+static inline void outl(uint32_t value, unsigned long port)
+{
+	writel(value, (volatile void __iomem *)port);
+}
+#endif
+
 #ifndef ioremap
 static inline void __iomem *ioremap(phys_addr_t phys_addr, size_t size __unused)
 {
diff --git a/lib/x86/asm/io.h b/lib/x86/asm/io.h
index 2436822162de..35a5c7347411 100644
--- a/lib/x86/asm/io.h
+++ b/lib/x86/asm/io.h
@@ -3,52 +3,63 @@
 
 #define __iomem
 
-static inline unsigned char inb(unsigned short port)
+#define inb inb
+static inline uint8_t inb(unsigned long port)
 {
     unsigned char value;
-    asm volatile("inb %w1, %0" : "=a" (value) : "Nd" (port));
+    asm volatile("inb %w1, %0" : "=a" (value) : "Nd" ((unsigned short)port));
     return value;
 }
 
-static inline unsigned short inw(unsigned short port)
+#define inw inw
+static inline uint16_t inw(unsigned long port)
 {
     unsigned short value;
-    asm volatile("inw %w1, %0" : "=a" (value) : "Nd" (port));
+    asm volatile("inw %w1, %0" : "=a" (value) : "Nd" ((unsigned short)port));
     return value;
 }
 
-static inline unsigned int inl(unsigned short port)
+#define inl inl
+static inline uint32_t inl(unsigned long port)
 {
     unsigned int value;
-    asm volatile("inl %w1, %0" : "=a" (value) : "Nd" (port));
+    asm volatile("inl %w1, %0" : "=a" (value) : "Nd" ((unsigned short)port));
     return value;
 }
 
-static inline void outb(unsigned char value, unsigned short port)
+#define outb outb
+static inline void outb(uint8_t value, unsigned long port)
 {
-    asm volatile("outb %b0, %w1" : : "a"(value), "Nd"(port));
+    asm volatile("outb %b0, %w1" : : "a"(value), "Nd"((unsigned short)port));
 }
 
-static inline void outw(unsigned short value, unsigned short port)
+#define outw outw
+static inline void outw(uint16_t value, unsigned long port)
 {
-    asm volatile("outw %w0, %w1" : : "a"(value), "Nd"(port));
+    asm volatile("outw %w0, %w1" : : "a"(value), "Nd"((unsigned short)port));
 }
 
-static inline void outl(unsigned int value, unsigned short port)
+#define outl outl
+static inline void outl(uint32_t value, unsigned long port)
 {
-    asm volatile("outl %0, %w1" : : "a"(value), "Nd"(port));
+    asm volatile("outl %0, %w1" : : "a"(value), "Nd"((unsigned short)port));
 }
 
+#define virt_to_phys virt_to_phys
 static inline unsigned long virt_to_phys(const void *virt)
 {
     return (unsigned long)virt;
 }
 
+#define phys_to_virt phys_to_virt
 static inline void *phys_to_virt(unsigned long phys)
 {
     return (void *)phys;
 }
 
+#define ioremap ioremap
 void __iomem *ioremap(phys_addr_t phys_addr, size_t size);
 
+#include <asm-generic/io.h>
+
 #endif
-- 
1.8.3.1

Re: [kvm-unit-tests PATCH v9 13/12] io: Unify IO accessors across architectures

[Andrew Jones]

On Fri, Jun 10, 2016 at 08:37:45AM +0200, Alexander Gordeev wrote:
> Cc: Andrew Jones <drjones@redhat.com>
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> 
> Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
> ---
>  lib/asm-generic/io.h | 43 +++++++++++++++++++++++++++++++++++++++++++
>  lib/x86/asm/io.h     | 35 +++++++++++++++++++++++------------
>  2 files changed, 66 insertions(+), 12 deletions(-)
> 
> diff --git a/lib/asm-generic/io.h b/lib/asm-generic/io.h
> index 842868ab9d58..91a2d7995d3d 100644
> --- a/lib/asm-generic/io.h
> +++ b/lib/asm-generic/io.h
> @@ -11,6 +11,7 @@
>   */
>  #include "libcflat.h"
>  #include "asm/page.h"
> +#include "asm/barrier.h"

This should have been in the last patch. I was going to
say that it doesn't break compilation between the commits,
because all includers of asm-generic/io.h are including
asm/barrier.h first, but then I just looked at ppc64.
I think if you try to compile the last commit without this
commit for powerpc, it'll break. So this should be fixed.

>  
>  #ifndef __raw_readb
>  static inline u8 __raw_readb(const volatile void *addr)
> @@ -145,6 +146,48 @@ static inline u64 __bswap64(u64 x)
>  #define writeq(b, addr) \
>  	({ wmb(); __raw_writeq(cpu_to_le64(b), addr); })
>  
> +#ifndef inb
> +static inline uint8_t inb(unsigned long port)
> +{
> +	return readb((const volatile void __iomem *)port);
> +}
> +#endif
> +
> +#ifndef inw
> +static inline uint16_t inw(unsigned long port)
> +{
> +	return readw((const volatile void __iomem *)port);
> +}
> +#endif
> +
> +#ifndef inl
> +static inline uint32_t inl(unsigned long port)
> +{
> +	return readl((const volatile void __iomem *)port);
> +}
> +#endif
> +
> +#ifndef outb
> +static inline void outb(uint8_t value, unsigned long port)
> +{
> +	writeb(value, (volatile void __iomem *)port);
> +}
> +#endif
> +
> +#ifndef outw
> +static inline void outw(uint16_t value, unsigned long port)
> +{
> +	writew(value, (volatile void __iomem *)port);
> +}
> +#endif
> +
> +#ifndef outl
> +static inline void outl(uint32_t value, unsigned long port)
> +{
> +	writel(value, (volatile void __iomem *)port);
> +}
> +#endif
> +
>  #ifndef ioremap
>  static inline void __iomem *ioremap(phys_addr_t phys_addr, size_t size __unused)
>  {
> diff --git a/lib/x86/asm/io.h b/lib/x86/asm/io.h
> index 2436822162de..35a5c7347411 100644
> --- a/lib/x86/asm/io.h
> +++ b/lib/x86/asm/io.h
> @@ -3,52 +3,63 @@
>  
>  #define __iomem
>  
> -static inline unsigned char inb(unsigned short port)
> +#define inb inb
> +static inline uint8_t inb(unsigned long port)
>  {
>      unsigned char value;
> -    asm volatile("inb %w1, %0" : "=a" (value) : "Nd" (port));
> +    asm volatile("inb %w1, %0" : "=a" (value) : "Nd" ((unsigned short)port));
>      return value;
>  }
>  
> -static inline unsigned short inw(unsigned short port)
> +#define inw inw
> +static inline uint16_t inw(unsigned long port)
>  {
>      unsigned short value;
> -    asm volatile("inw %w1, %0" : "=a" (value) : "Nd" (port));
> +    asm volatile("inw %w1, %0" : "=a" (value) : "Nd" ((unsigned short)port));
>      return value;
>  }
>  
> -static inline unsigned int inl(unsigned short port)
> +#define inl inl
> +static inline uint32_t inl(unsigned long port)
>  {
>      unsigned int value;
> -    asm volatile("inl %w1, %0" : "=a" (value) : "Nd" (port));
> +    asm volatile("inl %w1, %0" : "=a" (value) : "Nd" ((unsigned short)port));
>      return value;
>  }
>  
> -static inline void outb(unsigned char value, unsigned short port)
> +#define outb outb
> +static inline void outb(uint8_t value, unsigned long port)
>  {
> -    asm volatile("outb %b0, %w1" : : "a"(value), "Nd"(port));
> +    asm volatile("outb %b0, %w1" : : "a"(value), "Nd"((unsigned short)port));
>  }
>  
> -static inline void outw(unsigned short value, unsigned short port)
> +#define outw outw
> +static inline void outw(uint16_t value, unsigned long port)
>  {
> -    asm volatile("outw %w0, %w1" : : "a"(value), "Nd"(port));
> +    asm volatile("outw %w0, %w1" : : "a"(value), "Nd"((unsigned short)port));
>  }
>  
> -static inline void outl(unsigned int value, unsigned short port)
> +#define outl outl
> +static inline void outl(uint32_t value, unsigned long port)
>  {
> -    asm volatile("outl %0, %w1" : : "a"(value), "Nd"(port));
> +    asm volatile("outl %0, %w1" : : "a"(value), "Nd"((unsigned short)port));
>  }
>  
> +#define virt_to_phys virt_to_phys
>  static inline unsigned long virt_to_phys(const void *virt)
>  {
>      return (unsigned long)virt;
>  }
>  
> +#define phys_to_virt phys_to_virt
>  static inline void *phys_to_virt(unsigned long phys)
>  {
>      return (void *)phys;
>  }
>  
> +#define ioremap ioremap
>  void __iomem *ioremap(phys_addr_t phys_addr, size_t size);
>  
> +#include <asm-generic/io.h>
> +
>  #endif
> -- 
> 1.8.3.1
>

Otherwise looks good.

drew 

Re: [kvm-unit-tests PATCH v9 13/12] io: Unify IO accessors across architectures

[Alexander Gordeev]

On Fri, Jun 10, 2016 at 09:07:01AM +0200, Andrew Jones wrote:
> > diff --git a/lib/asm-generic/io.h b/lib/asm-generic/io.h
> > index 842868ab9d58..91a2d7995d3d 100644
> > --- a/lib/asm-generic/io.h
> > +++ b/lib/asm-generic/io.h
> > @@ -11,6 +11,7 @@
> >   */
> >  #include "libcflat.h"
> >  #include "asm/page.h"
> > +#include "asm/barrier.h"
> 
> This should have been in the last patch. I was going to
> say that it doesn't break compilation between the commits,
> because all includers of asm-generic/io.h are including
> asm/barrier.h first, but then I just looked at ppc64.
> I think if you try to compile the last commit without this
> commit for powerpc, it'll break. So this should be fixed.

Actually it is not, but I guess due to ppc64 does not call
these macros. Will repost shortly.

> drew 

[kvm-unit-tests PATCH v10 13/12] io: Unify IO accessors across architectures

[Alexander Gordeev]

Cc: Andrew Jones <drjones@redhat.com>
Cc: Thomas Huth <thuth@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>

Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
 lib/asm-generic/io.h | 42 ++++++++++++++++++++++++++++++++++++++++++
 lib/x86/asm/io.h     | 35 +++++++++++++++++++++++------------
 2 files changed, 65 insertions(+), 12 deletions(-)

diff --git a/lib/asm-generic/io.h b/lib/asm-generic/io.h
index 24725c3cd786..91a2d7995d3d 100644
--- a/lib/asm-generic/io.h
+++ b/lib/asm-generic/io.h
@@ -146,6 +146,48 @@ static inline u64 __bswap64(u64 x)
 #define writeq(b, addr) \
 	({ wmb(); __raw_writeq(cpu_to_le64(b), addr); })
 
+#ifndef inb
+static inline uint8_t inb(unsigned long port)
+{
+	return readb((const volatile void __iomem *)port);
+}
+#endif
+
+#ifndef inw
+static inline uint16_t inw(unsigned long port)
+{
+	return readw((const volatile void __iomem *)port);
+}
+#endif
+
+#ifndef inl
+static inline uint32_t inl(unsigned long port)
+{
+	return readl((const volatile void __iomem *)port);
+}
+#endif
+
+#ifndef outb
+static inline void outb(uint8_t value, unsigned long port)
+{
+	writeb(value, (volatile void __iomem *)port);
+}
+#endif
+
+#ifndef outw
+static inline void outw(uint16_t value, unsigned long port)
+{
+	writew(value, (volatile void __iomem *)port);
+}
+#endif
+
+#ifndef outl
+static inline void outl(uint32_t value, unsigned long port)
+{
+	writel(value, (volatile void __iomem *)port);
+}
+#endif
+
 #ifndef ioremap
 static inline void __iomem *ioremap(phys_addr_t phys_addr, size_t size __unused)
 {
diff --git a/lib/x86/asm/io.h b/lib/x86/asm/io.h
index 2436822162de..35a5c7347411 100644
--- a/lib/x86/asm/io.h
+++ b/lib/x86/asm/io.h
@@ -3,52 +3,63 @@
 
 #define __iomem
 
-static inline unsigned char inb(unsigned short port)
+#define inb inb
+static inline uint8_t inb(unsigned long port)
 {
     unsigned char value;
-    asm volatile("inb %w1, %0" : "=a" (value) : "Nd" (port));
+    asm volatile("inb %w1, %0" : "=a" (value) : "Nd" ((unsigned short)port));
     return value;
 }
 
-static inline unsigned short inw(unsigned short port)
+#define inw inw
+static inline uint16_t inw(unsigned long port)
 {
     unsigned short value;
-    asm volatile("inw %w1, %0" : "=a" (value) : "Nd" (port));
+    asm volatile("inw %w1, %0" : "=a" (value) : "Nd" ((unsigned short)port));
     return value;
 }
 
-static inline unsigned int inl(unsigned short port)
+#define inl inl
+static inline uint32_t inl(unsigned long port)
 {
     unsigned int value;
-    asm volatile("inl %w1, %0" : "=a" (value) : "Nd" (port));
+    asm volatile("inl %w1, %0" : "=a" (value) : "Nd" ((unsigned short)port));
     return value;
 }
 
-static inline void outb(unsigned char value, unsigned short port)
+#define outb outb
+static inline void outb(uint8_t value, unsigned long port)
 {
-    asm volatile("outb %b0, %w1" : : "a"(value), "Nd"(port));
+    asm volatile("outb %b0, %w1" : : "a"(value), "Nd"((unsigned short)port));
 }
 
-static inline void outw(unsigned short value, unsigned short port)
+#define outw outw
+static inline void outw(uint16_t value, unsigned long port)
 {
-    asm volatile("outw %w0, %w1" : : "a"(value), "Nd"(port));
+    asm volatile("outw %w0, %w1" : : "a"(value), "Nd"((unsigned short)port));
 }
 
-static inline void outl(unsigned int value, unsigned short port)
+#define outl outl
+static inline void outl(uint32_t value, unsigned long port)
 {
-    asm volatile("outl %0, %w1" : : "a"(value), "Nd"(port));
+    asm volatile("outl %0, %w1" : : "a"(value), "Nd"((unsigned short)port));
 }
 
+#define virt_to_phys virt_to_phys
 static inline unsigned long virt_to_phys(const void *virt)
 {
     return (unsigned long)virt;
 }
 
+#define phys_to_virt phys_to_virt
 static inline void *phys_to_virt(unsigned long phys)
 {
     return (void *)phys;
 }
 
+#define ioremap ioremap
 void __iomem *ioremap(phys_addr_t phys_addr, size_t size);
 
+#include <asm-generic/io.h>
+
 #endif
-- 
1.8.3.1

Re: [kvm-unit-tests PATCH v10 13/12] io: Unify IO accessors across architectures

[Andrew Jones]

On Fri, Jun 10, 2016 at 10:17:56AM +0200, Alexander Gordeev wrote:
> Cc: Andrew Jones <drjones@redhat.com>
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> 
> Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
> ---
>  lib/asm-generic/io.h | 42 ++++++++++++++++++++++++++++++++++++++++++
>  lib/x86/asm/io.h     | 35 +++++++++++++++++++++++------------
>  2 files changed, 65 insertions(+), 12 deletions(-)
> 
> diff --git a/lib/asm-generic/io.h b/lib/asm-generic/io.h
> index 24725c3cd786..91a2d7995d3d 100644
> --- a/lib/asm-generic/io.h
> +++ b/lib/asm-generic/io.h
> @@ -146,6 +146,48 @@ static inline u64 __bswap64(u64 x)
>  #define writeq(b, addr) \
>  	({ wmb(); __raw_writeq(cpu_to_le64(b), addr); })
>  
> +#ifndef inb
> +static inline uint8_t inb(unsigned long port)
> +{
> +	return readb((const volatile void __iomem *)port);
> +}
> +#endif
> +
> +#ifndef inw
> +static inline uint16_t inw(unsigned long port)
> +{
> +	return readw((const volatile void __iomem *)port);
> +}
> +#endif
> +
> +#ifndef inl
> +static inline uint32_t inl(unsigned long port)
> +{
> +	return readl((const volatile void __iomem *)port);
> +}
> +#endif
> +
> +#ifndef outb
> +static inline void outb(uint8_t value, unsigned long port)
> +{
> +	writeb(value, (volatile void __iomem *)port);
> +}
> +#endif
> +
> +#ifndef outw
> +static inline void outw(uint16_t value, unsigned long port)
> +{
> +	writew(value, (volatile void __iomem *)port);
> +}
> +#endif
> +
> +#ifndef outl
> +static inline void outl(uint32_t value, unsigned long port)
> +{
> +	writel(value, (volatile void __iomem *)port);
> +}
> +#endif
> +
>  #ifndef ioremap
>  static inline void __iomem *ioremap(phys_addr_t phys_addr, size_t size __unused)
>  {
> diff --git a/lib/x86/asm/io.h b/lib/x86/asm/io.h
> index 2436822162de..35a5c7347411 100644
> --- a/lib/x86/asm/io.h
> +++ b/lib/x86/asm/io.h
> @@ -3,52 +3,63 @@
>  
>  #define __iomem
>  
> -static inline unsigned char inb(unsigned short port)
> +#define inb inb
> +static inline uint8_t inb(unsigned long port)
>  {
>      unsigned char value;
> -    asm volatile("inb %w1, %0" : "=a" (value) : "Nd" (port));
> +    asm volatile("inb %w1, %0" : "=a" (value) : "Nd" ((unsigned short)port));
>      return value;
>  }
>  
> -static inline unsigned short inw(unsigned short port)
> +#define inw inw
> +static inline uint16_t inw(unsigned long port)
>  {
>      unsigned short value;
> -    asm volatile("inw %w1, %0" : "=a" (value) : "Nd" (port));
> +    asm volatile("inw %w1, %0" : "=a" (value) : "Nd" ((unsigned short)port));
>      return value;
>  }
>  
> -static inline unsigned int inl(unsigned short port)
> +#define inl inl
> +static inline uint32_t inl(unsigned long port)
>  {
>      unsigned int value;
> -    asm volatile("inl %w1, %0" : "=a" (value) : "Nd" (port));
> +    asm volatile("inl %w1, %0" : "=a" (value) : "Nd" ((unsigned short)port));
>      return value;
>  }
>  
> -static inline void outb(unsigned char value, unsigned short port)
> +#define outb outb
> +static inline void outb(uint8_t value, unsigned long port)
>  {
> -    asm volatile("outb %b0, %w1" : : "a"(value), "Nd"(port));
> +    asm volatile("outb %b0, %w1" : : "a"(value), "Nd"((unsigned short)port));
>  }
>  
> -static inline void outw(unsigned short value, unsigned short port)
> +#define outw outw
> +static inline void outw(uint16_t value, unsigned long port)
>  {
> -    asm volatile("outw %w0, %w1" : : "a"(value), "Nd"(port));
> +    asm volatile("outw %w0, %w1" : : "a"(value), "Nd"((unsigned short)port));
>  }
>  
> -static inline void outl(unsigned int value, unsigned short port)
> +#define outl outl
> +static inline void outl(uint32_t value, unsigned long port)
>  {
> -    asm volatile("outl %0, %w1" : : "a"(value), "Nd"(port));
> +    asm volatile("outl %0, %w1" : : "a"(value), "Nd"((unsigned short)port));
>  }
>  
> +#define virt_to_phys virt_to_phys
>  static inline unsigned long virt_to_phys(const void *virt)
>  {
>      return (unsigned long)virt;
>  }
>  
> +#define phys_to_virt phys_to_virt
>  static inline void *phys_to_virt(unsigned long phys)
>  {
>      return (void *)phys;
>  }
>  
> +#define ioremap ioremap
>  void __iomem *ioremap(phys_addr_t phys_addr, size_t size);
>  
> +#include <asm-generic/io.h>

I'm not sure it's necessary to include this for x86, but it
doesn't hurt either, and does do what this series wants to
do, that is to make x86 more consistent with the other arches.

Reviewed-by: Andrew Jones <drjones@redhat.com>

> +
>  #endif
> -- 
> 1.8.3.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [kvm-unit-tests PATCH v9 13/12] io: Unify IO accessors across architectures

[Paolo Bonzini]

On 10/06/2016 08:37, Alexander Gordeev wrote:
> diff --git a/lib/asm-generic/io.h b/lib/asm-generic/io.h
> index 842868ab9d58..91a2d7995d3d 100644
> --- a/lib/asm-generic/io.h
> +++ b/lib/asm-generic/io.h
> @@ -11,6 +11,7 @@
>   */
>  #include "libcflat.h"
>  #include "asm/page.h"
> +#include "asm/barrier.h"

There is no reason to include asm/barrier.h here.  Please remove this
indirect inclusion.

I'm applying patches 1-11.

Thanks,

Paolo

>  
>  #ifndef __raw_readb

Re: [kvm-unit-tests PATCH v9 13/12] io: Unify IO accessors across architectures

[Andrew Jones]

On Mon, Jun 13, 2016 at 03:35:43PM +0200, Paolo Bonzini wrote:
> 
> 
> On 10/06/2016 08:37, Alexander Gordeev wrote:
> > diff --git a/lib/asm-generic/io.h b/lib/asm-generic/io.h
> > index 842868ab9d58..91a2d7995d3d 100644
> > --- a/lib/asm-generic/io.h
> > +++ b/lib/asm-generic/io.h
> > @@ -11,6 +11,7 @@
> >   */
> >  #include "libcflat.h"
> >  #include "asm/page.h"
> > +#include "asm/barrier.h"
> 
> There is no reason to include asm/barrier.h here.  Please remove this
> indirect inclusion.

lib/asm-generic/io.h has barrier use. Without including it here,
then all includers of lib/asm-generic/io.h must include
asm/barrier.h first, even if they're unaware of the barrier use.

drew

Re: [kvm-unit-tests PATCH v9 13/12] io: Unify IO accessors across architectures

[Paolo Bonzini]

On 13/06/2016 15:57, Andrew Jones wrote:
>> > 
>> > There is no reason to include asm/barrier.h here.  Please remove this
>> > indirect inclusion.
> lib/asm-generic/io.h has barrier use. Without including it here,
> then all includers of lib/asm-generic/io.h must include
> asm/barrier.h first, even if they're unaware of the barrier use.

Are the barriers really necessary?  I.e. why do we need
__raw_{read,write}* vs just {read,write}*.

Paolo

Re: [kvm-unit-tests PATCH v9 13/12] io: Unify IO accessors across architectures

[Andrew Jones]

On Mon, Jun 13, 2016 at 04:03:53PM +0200, Paolo Bonzini wrote:
> 
> 
> On 13/06/2016 15:57, Andrew Jones wrote:
> >> > 
> >> > There is no reason to include asm/barrier.h here.  Please remove this
> >> > indirect inclusion.
> > lib/asm-generic/io.h has barrier use. Without including it here,
> > then all includers of lib/asm-generic/io.h must include
> > asm/barrier.h first, even if they're unaware of the barrier use.
> 
> Are the barriers really necessary?  I.e. why do we need
> __raw_{read,write}* vs just {read,write}*.

We could drop the wrappers, and require unit tests to use barriers
explicitly, when needed. I'm OK with that, because I think I mostly
forget that we have them in the wrappers and use them explicitly
anyway...  I'll need to double check all the uses though.

Thanks,
drew

Re: [kvm-unit-tests PATCH v9 13/12] io: Unify IO accessors across architectures

[Alexander Gordeev]

On Mon, Jun 13, 2016 at 04:17:19PM +0200, Andrew Jones wrote:
> On Mon, Jun 13, 2016 at 04:03:53PM +0200, Paolo Bonzini wrote:
> > Are the barriers really necessary?  I.e. why do we need
> > __raw_{read,write}* vs just {read,write}*.
> 
> We could drop the wrappers, and require unit tests to use barriers
> explicitly, when needed. I'm OK with that, because I think I mostly
> forget that we have them in the wrappers and use them explicitly
> anyway...  I'll need to double check all the uses though.

According to my findings read*/write* io accessors are used on
ioremap-ed areas in lib/arm/io.c and lib/virtio-mmio.c at the moment.

Even if read*/write* wrappers are stripped of SMP barriers,
removing compiler barriers from them would be a bad idea AFAICT.

So if we need compiler barriers in read*/write* then why not
just leave SMP barriers there?

> Thanks,
> drew

Re: [kvm-unit-tests PATCH v9 13/12] io: Unify IO accessors across architectures

[Alexander Gordeev]

On Wed, Jun 29, 2016 at 10:24:16AM +0200, Alexander Gordeev wrote:
> On Mon, Jun 13, 2016 at 04:17:19PM +0200, Andrew Jones wrote:
> > On Mon, Jun 13, 2016 at 04:03:53PM +0200, Paolo Bonzini wrote:
> > > Are the barriers really necessary?  I.e. why do we need
> > > __raw_{read,write}* vs just {read,write}*.
> > 
> > We could drop the wrappers, and require unit tests to use barriers
> > explicitly, when needed. I'm OK with that, because I think I mostly
> > forget that we have them in the wrappers and use them explicitly
> > anyway...  I'll need to double check all the uses though.
> 
> According to my findings read*/write* io accessors are used on
> ioremap-ed areas in lib/arm/io.c and lib/virtio-mmio.c at the moment.
> 
> Even if read*/write* wrappers are stripped of SMP barriers,
> removing compiler barriers from them would be a bad idea AFAICT.
> 
> So if we need compiler barriers in read*/write* then why not
> just leave SMP barriers there?

Andrew, Paolo,

I am paraphrasing myself. So read*/write* are used extensively
by lib/virtio-mmio.c to access memory mapped registers. AFAICT
we do want serialize accesses to devices and need barriers for
that. The same applies to lib/pci-testdev.c when/if it accepted.

> > Thanks,
> > drew

Re: [kvm-unit-tests PATCH v9 13/12] io: Unify IO accessors across architectures

[Paolo Bonzini]

On 09/07/2016 22:39, Alexander Gordeev wrote:
> > > We could drop the wrappers, and require unit tests to use barriers
> > > explicitly, when needed. I'm OK with that, because I think I mostly
> > > forget that we have them in the wrappers and use them explicitly
> > > anyway...  I'll need to double check all the uses though.
> > 
> > According to my findings read*/write* io accessors are used on
> > ioremap-ed areas in lib/arm/io.c and lib/virtio-mmio.c at the moment.
> > 
> > Even if read*/write* wrappers are stripped of SMP barriers,
> > removing compiler barriers from them would be a bad idea AFAICT.

I agree (though ACCESS_ONCE-style volatile access will do).

> > So if we need compiler barriers in read*/write* then why not
> > just leave SMP barriers there?

Fair enough, I'm applying the rest of the series.

Paolo

> Andrew, Paolo,
> 
> I am paraphrasing myself. So read*/write* are used extensively
> by lib/virtio-mmio.c to access memory mapped registers. AFAICT
> we do want serialize accesses to devices and need barriers for
> that. The same applies to lib/pci-testdev.c when/if it accepted.
> 

Re: [kvm-unit-tests PATCH v9 13/12] io: Unify IO accessors across architectures

[Alexander Gordeev]

On Mon, Jul 11, 2016 at 05:01:36PM +0200, Paolo Bonzini wrote:
> Fair enough, I'm applying the rest of the series.

Oops, I did not expect it that fast :) I was going to send an
updated version which considers Andrew's comments and your
clarification of pci-testdev writes in qemu-devel. But I can
send follow-up fixes surely.

> Paolo

Re: [kvm-unit-tests PATCH v9 13/12] io: Unify IO accessors across architectures

[Paolo Bonzini]

On 11/07/2016 18:17, Alexander Gordeev wrote:
>> > Fair enough, I'm applying the rest of the series.
> Oops, I did not expect it that fast :) I was going to send an
> updated version which considers Andrew's comments and your
> clarification of pci-testdev writes in qemu-devel. But I can
> send follow-up fixes surely.

Yeah, let's go ahead with follow up patches.

Paolo

Re: [kvm-unit-tests PATCH v9 13/12] io: Unify IO accessors across architectures

[Andrew Jones]

On Mon, Jul 11, 2016 at 06:17:12PM +0200, Alexander Gordeev wrote:
> On Mon, Jul 11, 2016 at 05:01:36PM +0200, Paolo Bonzini wrote:
> > Fair enough, I'm applying the rest of the series.
> 
> Oops, I did not expect it that fast :) I was going to send an
> updated version which considers Andrew's comments and your
> clarification of pci-testdev writes in qemu-devel. But I can
> send follow-up fixes surely.

Paolo just meant the remaining two patches of this series, which
doesn't address pci-testdev. Your other series, bringing pci-
testdev to arm, is still under review. Please post a new version
of that rebased on latest master.

Thanks,
drew


> 
> > Paolo
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html