* [PATCH] ARM64: dts: meson-gx: fix ATF reserved memory region
From: Kevin Hilman @ 2018-06-08 23:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180607205514.36631-1-khilman@baylibre.com>
Kevin Hilman <khilman@baylibre.com> writes:
> Vendor firmware/uboot has different reserved regions depending on
> firmware version, but current codebase reserves the same regions on
> GXL and GXBB, so move the additional reserved memory region to common
> .dtsi.
>
> Found when putting a recent vendor u-boot on meson-gxbb-p200.
>
> Recommended-by: Neil Armstrong <narmstrong@baylibre.com>
oops, this should've been the more common "Suggested-by". Fixed, and
then...
> Signed-off-by: Kevin Hilman <khilman@baylibre.com>
...added a Cc:stable and applied to v4.18/fixes.
Kevin
> ---
> arch/arm64/boot/dts/amlogic/meson-gx.dtsi | 6 ++++++
> arch/arm64/boot/dts/amlogic/meson-gxl.dtsi | 8 --------
> 2 files changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
> index b003f324ca31..b8dc4dbb391b 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-gx.dtsi
> @@ -35,6 +35,12 @@
> no-map;
> };
>
> + /* Alternate 3 MiB reserved for ARM Trusted Firmware (BL31) */
> + secmon_reserved_alt: secmon at 5000000 {
> + reg = <0x0 0x05000000 0x0 0x300000>;
> + no-map;
> + };
> +
> linux,cma {
> compatible = "shared-dma-pool";
> reusable;
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
> index 27538eea547b..c87a80e9bcc6 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxl.dtsi
> @@ -13,14 +13,6 @@
> / {
> compatible = "amlogic,meson-gxl";
>
> - reserved-memory {
> - /* Alternate 3 MiB reserved for ARM Trusted Firmware (BL31) */
> - secmon_reserved_alt: secmon at 5000000 {
> - reg = <0x0 0x05000000 0x0 0x300000>;
> - no-map;
> - };
> - };
> -
> soc {
> usb0: usb at c9000000 {
> status = "disabled";
^ permalink raw reply
* [PATCH 04/24] 32-bit userspace ABI: introduce ARCH_32BIT_OFF_T config option
From: Yury Norov @ 2018-06-09 7:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180608173207.nwoi25jee52gpdwy@armageddon.cambridge.arm.com>
On Fri, Jun 08, 2018 at 06:32:07PM +0100, Catalin Marinas wrote:
> On Wed, May 16, 2018 at 11:18:49AM +0300, Yury Norov wrote:
> > diff --git a/arch/Kconfig b/arch/Kconfig
> > index 76c0b54443b1..ee079244dc3c 100644
> > --- a/arch/Kconfig
> > +++ b/arch/Kconfig
> > @@ -264,6 +264,21 @@ config ARCH_THREAD_STACK_ALLOCATOR
> > config ARCH_WANTS_DYNAMIC_TASK_STRUCT
> > bool
> >
> > +config ARCH_32BIT_OFF_T
> > + bool
> > + depends on !64BIT
> > + help
> > + All new 32-bit architectures should have 64-bit off_t type on
> > + userspace side which corresponds to the loff_t kernel type. This
> > + is the requirement for modern ABIs. Some existing architectures
> > + already have 32-bit off_t. This option is enabled for all such
> > + architectures explicitly. Namely: arc, arm, blackfin, cris, frv,
> > + h8300, hexagon, m32r, m68k, metag, microblaze, mips32, mn10300,
> > + nios2, openrisc, parisc32, powerpc32, score, sh, sparc, tile32,
> > + unicore32, x86_32 and xtensa. This is the complete list. Any
> > + new 32-bit architecture should declare 64-bit off_t type on user
> > + side and so should not enable this option.
>
> Do you know if this is the case for riscv and nds32, merged in the
> meantime? If not, I suggest you drop this patch altogether and just
> define force_o_largefile() for arm64/ilp32 as we don't seem to stick to
> "all new 32-bit architectures should have 64-bit off_t".
I wrote this patch at request of Arnd Bergmann. This is actually his
words that all new 32-bit architectures should have 64-bit off_t. So
I was surprized when riscv was merged with 32-bit off_t (and I didn't
follow nds32).
If this rule is still in force, we'd better add new exceptions to this
patch. Otherwise, we can drop it.
Arnd, could you please comment it?
Yury
^ permalink raw reply
* [PATCH 04/24] 32-bit userspace ABI: introduce ARCH_32BIT_OFF_T config option
From: Yury Norov @ 2018-06-09 7:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <mhng-e1922456-a05b-46f9-8644-d45ad70a55e5@palmer-si-x1c4>
On Fri, Jun 08, 2018 at 03:33:51PM -0700, Palmer Dabbelt wrote:
> On Fri, 08 Jun 2018 10:32:07 PDT (-0700), catalin.marinas at arm.com wrote:
> > On Wed, May 16, 2018 at 11:18:49AM +0300, Yury Norov wrote:
> > > diff --git a/arch/Kconfig b/arch/Kconfig
> > > index 76c0b54443b1..ee079244dc3c 100644
> > > --- a/arch/Kconfig
> > > +++ b/arch/Kconfig
> > > @@ -264,6 +264,21 @@ config ARCH_THREAD_STACK_ALLOCATOR
> > > config ARCH_WANTS_DYNAMIC_TASK_STRUCT
> > > bool
> > >
> > > +config ARCH_32BIT_OFF_T
> > > + bool
> > > + depends on !64BIT
> > > + help
> > > + All new 32-bit architectures should have 64-bit off_t type on
> > > + userspace side which corresponds to the loff_t kernel type. This
> > > + is the requirement for modern ABIs. Some existing architectures
> > > + already have 32-bit off_t. This option is enabled for all such
> > > + architectures explicitly. Namely: arc, arm, blackfin, cris, frv,
> > > + h8300, hexagon, m32r, m68k, metag, microblaze, mips32, mn10300,
> > > + nios2, openrisc, parisc32, powerpc32, score, sh, sparc, tile32,
> > > + unicore32, x86_32 and xtensa. This is the complete list. Any
> > > + new 32-bit architecture should declare 64-bit off_t type on user
> > > + side and so should not enable this option.
> >
> > Do you know if this is the case for riscv and nds32, merged in the
> > meantime? If not, I suggest you drop this patch altogether and just
> > define force_o_largefile() for arm64/ilp32 as we don't seem to stick to
> > "all new 32-bit architectures should have 64-bit off_t".
>
> We (RISC-V) don't have support for rv32i in glibc yet, so there really isn't
> a fixed ABI there yet. From my understanding the rv32i port as it currently
> stands has a 32-bit off_t (via __kernel_off_t being defined as long), so
> this change would technically be a kernel ABI break.
>
> Since we don't have rv32i glibc yet I'm not fundamentally opposed to an ABI
> break. Is there a concrete advantage to this?
One obvious advantage is manipulating large files - if file is greater than
2G, you cannot easily mmap(), lseek() etc with 32-bit offset.
Another point is unification of layuots for structures like struct
stat between 32- and 64-bit worlds.
On glibc side it helps to unify 32-bit and 64-bit versions of syscalls.
Refer, for example this commit:
3c7f1f59cd161 (Consolidate lseek/lseek64/llseek implementations).
Yury
^ permalink raw reply
* [PATCH v2 2/6] arm64: KVM: Handle Set/Way CMOs as NOPs if FWB is present
From: Christoffer Dall @ 2018-06-09 9:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180530124706.25284-3-marc.zyngier@arm.com>
On Wed, May 30, 2018 at 01:47:02PM +0100, Marc Zyngier wrote:
> Set/Way handling is one of the ugliest corners of KVM. We shouldn't
> have to handle that, but better safe than sorry.
>
> Thankfully, FWB fixes this for us by not requiering any maintenance
> whatsoever, which means we don't have to emulate S/W CMOs, and don't
> have to track VM ops either.
I tiny bit of rationale here would have been nice. As I understand it,
if we're presenting the guest with a fully coherent system, there should
never be a need to invalidate anything, because the guest will always
see the most recent value no matter how it sings and dances, right?
>
> We still have to trap S/W though, if only to prevent the guest from
> doing something bad.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
> arch/arm64/kvm/sys_regs.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 6e3b969391fd..9a740f159245 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -195,7 +195,13 @@ static bool access_dcsw(struct kvm_vcpu *vcpu,
> if (!p->is_write)
> return read_from_write_only(vcpu, p, r);
>
> - kvm_set_way_flush(vcpu);
> + /*
> + * Only track S/W ops if we don't have FWB. It still indicates
> + * that the guest is a bit broken...
> + */
Is it strictly true that the guest is broken if it does any form of S/W
ops? Does the guest actually know that it's running on a fully coherent
system, or is the argument that no software, ever, should do S/W, even
for reboot etc.?
I think this should have slightly more info, or that part of the comment
should just be dropped, to avoid misleading future readers who don't
have the full picture.
> + if (!cpus_have_const_cap(ARM64_HAS_STAGE2_FWB))
> + kvm_set_way_flush(vcpu);
> +
> return true;
> }
>
> --
> 2.17.1
>
Besides the usual nits on commentary:
Reviewed-by: Christoffer Dall <christoffer.dall@arm.com>
^ permalink raw reply
* [PATCH v2 3/6] arm64: KVM: Avoid marking pages as XN in Stage-2 if CTR_EL0.DIC is set
From: Christoffer Dall @ 2018-06-09 9:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180530124706.25284-4-marc.zyngier@arm.com>
On Wed, May 30, 2018 at 01:47:03PM +0100, Marc Zyngier wrote:
> On systems where CTR_EL0.DIC is set, we don't need to perform
> icache invalidation to guarantee that we'll fetch the right
> instruction stream.
>
> This also means that taking a permission fault to invalidate the
> icache is an unnecessary overhead.
>
> On such systems, we can safely leave the page as being executable.
>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Acked-by: Christoffer Dall <christoffer.dall@arm.com>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
> arch/arm64/include/asm/pgtable-prot.h | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/include/asm/pgtable-prot.h b/arch/arm64/include/asm/pgtable-prot.h
> index c66c3047400e..78b942c1bea4 100644
> --- a/arch/arm64/include/asm/pgtable-prot.h
> +++ b/arch/arm64/include/asm/pgtable-prot.h
> @@ -77,8 +77,18 @@
> __val; \
> })
>
> -#define PAGE_S2 __pgprot(_PROT_DEFAULT | PAGE_S2_MEMATTR(NORMAL) | PTE_S2_RDONLY | PTE_S2_XN)
> -#define PAGE_S2_DEVICE __pgprot(_PROT_DEFAULT | PAGE_S2_MEMATTR(DEVICE_nGnRE) | PTE_S2_RDONLY | PTE_S2_XN)
> +#define PAGE_S2_XN \
> + ({ \
> + u64 __val; \
> + if (cpus_have_const_cap(ARM64_HAS_CACHE_DIC)) \
> + __val = 0; \
> + else \
> + __val = PTE_S2_XN; \
> + __val; \
> + })
> +
> +#define PAGE_S2 __pgprot(_PROT_DEFAULT | PAGE_S2_MEMATTR(NORMAL) | PTE_S2_RDONLY | PAGE_S2_XN)
> +#define PAGE_S2_DEVICE __pgprot(_PROT_DEFAULT | PAGE_S2_MEMATTR(DEVICE_nGnRE) | PTE_S2_RDONLY | PAGE_S2_XN)
>
> #define PAGE_NONE __pgprot(((_PAGE_DEFAULT) & ~PTE_VALID) | PTE_PROT_NONE | PTE_RDONLY | PTE_NG | PTE_PXN | PTE_UXN)
> #define PAGE_SHARED __pgprot(_PAGE_DEFAULT | PTE_USER | PTE_NG | PTE_PXN | PTE_UXN | PTE_WRITE)
> --
> 2.17.1
>
^ permalink raw reply
* [PATCH v2 4/6] KVM: arm/arm64: Consolidate page-table accessors
From: Christoffer Dall @ 2018-06-09 9:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180530124706.25284-5-marc.zyngier@arm.com>
On Wed, May 30, 2018 at 01:47:04PM +0100, Marc Zyngier wrote:
> The arm and arm64 KVM page tables accessors are pointlessly different
> between the two architectures, and likely both wrong one way or another:
> arm64 lacks a dsb(), and arm doesn't use WRITE_ONCE.
>
> Let's unify them.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
> arch/arm/include/asm/kvm_mmu.h | 12 -----------
> arch/arm64/include/asm/kvm_mmu.h | 3 ---
> virt/kvm/arm/mmu.c | 35 ++++++++++++++++++++++++++++----
> 3 files changed, 31 insertions(+), 19 deletions(-)
>
> diff --git a/arch/arm/include/asm/kvm_mmu.h b/arch/arm/include/asm/kvm_mmu.h
> index 707a1f06dc5d..468ff945efa0 100644
> --- a/arch/arm/include/asm/kvm_mmu.h
> +++ b/arch/arm/include/asm/kvm_mmu.h
> @@ -75,18 +75,6 @@ phys_addr_t kvm_get_idmap_vector(void);
> int kvm_mmu_init(void);
> void kvm_clear_hyp_idmap(void);
>
> -static inline void kvm_set_pmd(pmd_t *pmd, pmd_t new_pmd)
> -{
> - *pmd = new_pmd;
> - dsb(ishst);
> -}
> -
> -static inline void kvm_set_pte(pte_t *pte, pte_t new_pte)
> -{
> - *pte = new_pte;
> - dsb(ishst);
> -}
> -
> static inline pte_t kvm_s2pte_mkwrite(pte_t pte)
> {
> pte_val(pte) |= L_PTE_S2_RDWR;
> diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
> index 9dbca5355029..26c89b63f604 100644
> --- a/arch/arm64/include/asm/kvm_mmu.h
> +++ b/arch/arm64/include/asm/kvm_mmu.h
> @@ -170,9 +170,6 @@ phys_addr_t kvm_get_idmap_vector(void);
> int kvm_mmu_init(void);
> void kvm_clear_hyp_idmap(void);
>
> -#define kvm_set_pte(ptep, pte) set_pte(ptep, pte)
> -#define kvm_set_pmd(pmdp, pmd) set_pmd(pmdp, pmd)
> -
> static inline pte_t kvm_s2pte_mkwrite(pte_t pte)
> {
> pte_val(pte) |= PTE_S2_RDWR;
> diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
> index ba66bf7ae299..c9ed239c0840 100644
> --- a/virt/kvm/arm/mmu.c
> +++ b/virt/kvm/arm/mmu.c
> @@ -177,6 +177,33 @@ static void clear_stage2_pmd_entry(struct kvm *kvm, pmd_t *pmd, phys_addr_t addr
> put_page(virt_to_page(pmd));
> }
>
> +static inline void kvm_set_pte(pte_t *ptep, pte_t new_pte)
> +{
> + WRITE_ONCE(*ptep, new_pte);
> + dsb(ishst);
> +}
> +
> +static inline void kvm_set_pmd(pmd_t *pmdp, pmd_t new_pmd)
> +{
> + WRITE_ONCE(*pmdp, new_pmd);
> + dsb(ishst);
> +}
> +
arm64 set_pte and set_pmd have an isb() in addition to the dsb(), why
can we let go of that here?
> +static inline void kvm_pmd_populate(pmd_t *pmdp, pte_t *ptep)
> +{
> + pmd_populate_kernel(NULL, pmdp, ptep);
> +}
> +
> +static inline void kvm_pud_populate(pud_t *pudp, pmd_t *pmdp)
> +{
> + pud_populate(NULL, pudp, pmdp);
> +}
> +
> +static inline void kvm_pgd_populate(pgd_t *pgdp, pud_t *pudp)
> +{
> + pgd_populate(NULL, pgdp, pudp);
> +}
> +
> /*
> * Unmapping vs dcache management:
> *
> @@ -603,7 +630,7 @@ static int create_hyp_pmd_mappings(pud_t *pud, unsigned long start,
> kvm_err("Cannot allocate Hyp pte\n");
> return -ENOMEM;
> }
> - pmd_populate_kernel(NULL, pmd, pte);
> + kvm_pmd_populate(pmd, pte);
> get_page(virt_to_page(pmd));
> kvm_flush_dcache_to_poc(pmd, sizeof(*pmd));
> }
> @@ -636,7 +663,7 @@ static int create_hyp_pud_mappings(pgd_t *pgd, unsigned long start,
> kvm_err("Cannot allocate Hyp pmd\n");
> return -ENOMEM;
> }
> - pud_populate(NULL, pud, pmd);
> + kvm_pud_populate(pud, pmd);
> get_page(virt_to_page(pud));
> kvm_flush_dcache_to_poc(pud, sizeof(*pud));
> }
> @@ -673,7 +700,7 @@ static int __create_hyp_mappings(pgd_t *pgdp, unsigned long ptrs_per_pgd,
> err = -ENOMEM;
> goto out;
> }
> - pgd_populate(NULL, pgd, pud);
> + kvm_pgd_populate(pgd, pud);
> get_page(virt_to_page(pgd));
> kvm_flush_dcache_to_poc(pgd, sizeof(*pgd));
> }
> @@ -1092,7 +1119,7 @@ static int stage2_set_pte(struct kvm *kvm, struct kvm_mmu_memory_cache *cache,
> if (!cache)
> return 0; /* ignore calls from kvm_set_spte_hva */
> pte = mmu_memory_cache_alloc(cache);
> - pmd_populate_kernel(NULL, pmd, pte);
> + kvm_pmd_populate(pmd, pte);
> get_page(virt_to_page(pmd));
> }
>
> --
> 2.17.1
>
Otherwise:
Acked-by: Christoffer Dall <christoffer.dall@arm.com>
^ permalink raw reply
* [PATCH v2 5/6] KVM: arm/arm64: Stop using {pmd,pud,pgd}_populate
From: Christoffer Dall @ 2018-06-09 9:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180530124706.25284-6-marc.zyngier@arm.com>
On Wed, May 30, 2018 at 01:47:05PM +0100, Marc Zyngier wrote:
> The {pmd,pud,pgd}_populate accessors usage in the kernel have always
> been a bit weird in KVM. We don't have a struct mm to pass (and
> neither does the kernel most of the time, but still...), and
> the 32bit code has all kind of cache maintenance that doesn't make
> sense on ARMv7+ when MP extensions are mandatory (which is the
> case when the VEs are present).
>
> Let's bite the bullet and provide our own implementations. The
> only bit of architectural code left has to do with building the table
> entry itself (arm64 having up to 52bit PA, arm lacking PUD level).
>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Acked-by: Christoffer Dall <christoffer.dall@arm.com>
> ---
> arch/arm/include/asm/kvm_mmu.h | 4 ++++
> arch/arm64/include/asm/kvm_mmu.h | 7 +++++++
> virt/kvm/arm/mmu.c | 8 +++++---
> 3 files changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/include/asm/kvm_mmu.h b/arch/arm/include/asm/kvm_mmu.h
> index 468ff945efa0..a94ef9833bd3 100644
> --- a/arch/arm/include/asm/kvm_mmu.h
> +++ b/arch/arm/include/asm/kvm_mmu.h
> @@ -75,6 +75,10 @@ phys_addr_t kvm_get_idmap_vector(void);
> int kvm_mmu_init(void);
> void kvm_clear_hyp_idmap(void);
>
> +#define kvm_mk_pmd(ptep) __pmd(__pa(ptep) | PMD_TYPE_TABLE)
> +#define kvm_mk_pud(pmdp) __pud(__pa(pmdp) | PMD_TYPE_TABLE)
> +#define kvm_mk_pgd(pudp) ({ BUILD_BUG(); 0; })
> +
> static inline pte_t kvm_s2pte_mkwrite(pte_t pte)
> {
> pte_val(pte) |= L_PTE_S2_RDWR;
> diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
> index 26c89b63f604..22c9f7cfdf93 100644
> --- a/arch/arm64/include/asm/kvm_mmu.h
> +++ b/arch/arm64/include/asm/kvm_mmu.h
> @@ -170,6 +170,13 @@ phys_addr_t kvm_get_idmap_vector(void);
> int kvm_mmu_init(void);
> void kvm_clear_hyp_idmap(void);
>
> +#define kvm_mk_pmd(ptep) \
> + __pmd(__phys_to_pmd_val(__pa(ptep) | PMD_TYPE_TABLE))
> +#define kvm_mk_pud(pmdp) \
> + __pud(__phys_to_pud_val(__pa(pmdp) | PMD_TYPE_TABLE))
> +#define kvm_mk_pgd(pudp) \
> + __pgd(__phys_to_pgd_val(__pa(pudp) | PUD_TYPE_TABLE))
> +
> static inline pte_t kvm_s2pte_mkwrite(pte_t pte)
> {
> pte_val(pte) |= PTE_S2_RDWR;
> diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
> index c9ed239c0840..ad1980d2118a 100644
> --- a/virt/kvm/arm/mmu.c
> +++ b/virt/kvm/arm/mmu.c
> @@ -191,17 +191,19 @@ static inline void kvm_set_pmd(pmd_t *pmdp, pmd_t new_pmd)
>
> static inline void kvm_pmd_populate(pmd_t *pmdp, pte_t *ptep)
> {
> - pmd_populate_kernel(NULL, pmdp, ptep);
> + kvm_set_pmd(pmdp, kvm_mk_pmd(ptep));
> }
>
> static inline void kvm_pud_populate(pud_t *pudp, pmd_t *pmdp)
> {
> - pud_populate(NULL, pudp, pmdp);
> + WRITE_ONCE(*pudp, kvm_mk_pud(pmdp));
> + dsb(ishst);
> }
>
> static inline void kvm_pgd_populate(pgd_t *pgdp, pud_t *pudp)
> {
> - pgd_populate(NULL, pgdp, pudp);
> + WRITE_ONCE(*pgdp, kvm_mk_pgd(pudp));
> + dsb(ishst);
> }
>
> /*
> --
> 2.17.1
>
^ permalink raw reply
* [PATCH v2 6/6] KVM: arm/arm64: Remove unnecessary CMOs when creating HYP page tables
From: Christoffer Dall @ 2018-06-09 9:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180530124706.25284-7-marc.zyngier@arm.com>
On Wed, May 30, 2018 at 01:47:06PM +0100, Marc Zyngier wrote:
> There is no need to perform cache maintenance operations when
> creating the HYP page tables if we have the multiprocessing
> extensions. ARMv7 mandates them with the virtualization support,
> and ARMv8 just mandates them unconditionally.
>
> Let's remove these operations.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Acked-by: Christoffer Dall <christoffer.dall@arm.com>
> ---
> virt/kvm/arm/mmu.c | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
> index ad1980d2118a..ccdf544d44c0 100644
> --- a/virt/kvm/arm/mmu.c
> +++ b/virt/kvm/arm/mmu.c
> @@ -607,7 +607,6 @@ static void create_hyp_pte_mappings(pmd_t *pmd, unsigned long start,
> pte = pte_offset_kernel(pmd, addr);
> kvm_set_pte(pte, pfn_pte(pfn, prot));
> get_page(virt_to_page(pte));
> - kvm_flush_dcache_to_poc(pte, sizeof(*pte));
> pfn++;
> } while (addr += PAGE_SIZE, addr != end);
> }
> @@ -634,7 +633,6 @@ static int create_hyp_pmd_mappings(pud_t *pud, unsigned long start,
> }
> kvm_pmd_populate(pmd, pte);
> get_page(virt_to_page(pmd));
> - kvm_flush_dcache_to_poc(pmd, sizeof(*pmd));
> }
>
> next = pmd_addr_end(addr, end);
> @@ -667,7 +665,6 @@ static int create_hyp_pud_mappings(pgd_t *pgd, unsigned long start,
> }
> kvm_pud_populate(pud, pmd);
> get_page(virt_to_page(pud));
> - kvm_flush_dcache_to_poc(pud, sizeof(*pud));
> }
>
> next = pud_addr_end(addr, end);
> @@ -704,7 +701,6 @@ static int __create_hyp_mappings(pgd_t *pgdp, unsigned long ptrs_per_pgd,
> }
> kvm_pgd_populate(pgd, pud);
> get_page(virt_to_page(pgd));
> - kvm_flush_dcache_to_poc(pgd, sizeof(*pgd));
> }
>
> next = pgd_addr_end(addr, end);
> --
> 2.17.1
>
^ permalink raw reply
* [PATCH] KVM: arm/arm64: drop resource size check for GICV window
From: Christoffer Dall @ 2018-06-09 10:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180601150628.10111-1-ard.biesheuvel@linaro.org>
On Fri, Jun 01, 2018 at 05:06:28PM +0200, Ard Biesheuvel wrote:
> When booting a 64 KB pages kernel on a ACPI GICv3 system that
> implements support for v2 emulation, the following warning is
> produced
>
> GICV size 0x2000 not a multiple of page size 0x10000
>
> and support for v2 emulation is disabled, preventing GICv2 VMs
> from being able to run on such hosts.
>
> The reason is that vgic_v3_probe() performs a sanity check on the
> size of the window (it should be a multiple of the page size),
> while the ACPI MADT parsing code hardcodes the size of the window
> to 8 KB. This makes sense, considering that ACPI does not bother
> to describe the size in the first place, under the assumption that
> platforms implementing ACPI will follow the architecture and not
> put anything else in the same 64 KB window.
Does the architecture actually say that anywhere?
>
> So let's just drop the sanity check altogether, and assume that
> the window is at least 64 KB in size.
This could obviously be dangerous if broken systems actually exist.
Marc may know more about that than me. An alternative would be to
modify the ACPI code to assume max(8 KB, page size) instead, and/or a
command line parameter to override this check.
That said, I'm not directly opposed to this patch, but I'll let Marc
have a look as well.
Thanks,
-Christoffer
>
> Fixes: 909777324588 ("KVM: arm/arm64: vgic-new: vgic_init: implement kvm_vgic_hyp_init")
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> virt/kvm/arm/vgic/vgic-v3.c | 5 -----
> 1 file changed, 5 deletions(-)
>
> diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c
> index bdcf8e7a6161..72fc688c3e9d 100644
> --- a/virt/kvm/arm/vgic/vgic-v3.c
> +++ b/virt/kvm/arm/vgic/vgic-v3.c
> @@ -552,11 +552,6 @@ int vgic_v3_probe(const struct gic_kvm_info *info)
> pr_warn("GICV physical address 0x%llx not page aligned\n",
> (unsigned long long)info->vcpu.start);
> kvm_vgic_global_state.vcpu_base = 0;
> - } else if (!PAGE_ALIGNED(resource_size(&info->vcpu))) {
> - pr_warn("GICV size 0x%llx not a multiple of page size 0x%lx\n",
> - (unsigned long long)resource_size(&info->vcpu),
> - PAGE_SIZE);
> - kvm_vgic_global_state.vcpu_base = 0;
> } else {
> kvm_vgic_global_state.vcpu_base = info->vcpu.start;
> kvm_vgic_global_state.can_emulate_gicv2 = true;
> --
> 2.17.0
>
^ permalink raw reply
* [PATCH] KVM: arm/arm64: drop resource size check for GICV window
From: Ard Biesheuvel @ 2018-06-09 10:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180609100657.GI5097@C02W217FHV2R.local>
> On 9 Jun 2018, at 12:06, Christoffer Dall <christoffer.dall@arm.com> wrote:
>
>> On Fri, Jun 01, 2018 at 05:06:28PM +0200, Ard Biesheuvel wrote:
>> When booting a 64 KB pages kernel on a ACPI GICv3 system that
>> implements support for v2 emulation, the following warning is
>> produced
>>
>> GICV size 0x2000 not a multiple of page size 0x10000
>>
>> and support for v2 emulation is disabled, preventing GICv2 VMs
>> from being able to run on such hosts.
>>
>> The reason is that vgic_v3_probe() performs a sanity check on the
>> size of the window (it should be a multiple of the page size),
>> while the ACPI MADT parsing code hardcodes the size of the window
>> to 8 KB. This makes sense, considering that ACPI does not bother
>> to describe the size in the first place, under the assumption that
>> platforms implementing ACPI will follow the architecture and not
>> put anything else in the same 64 KB window.
>
> Does the architecture actually say that anywhere?
>
>>
>> So let's just drop the sanity check altogether, and assume that
>> the window is at least 64 KB in size.
>
> This could obviously be dangerous if broken systems actually exist.
> Marc may know more about that than me. An alternative would be to
> modify the ACPI code to assume max(8 KB, page size) instead, and/or a
> command line parameter to override this check.
>
> That said, I'm not directly opposed to this patch, but I'll let Marc
> have a look as well.
>
This approach was actually Marc?s idea, and he already applied the patch to the queue branch afaik.
>
>>
>> Fixes: 909777324588 ("KVM: arm/arm64: vgic-new: vgic_init: implement kvm_vgic_hyp_init")
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>> virt/kvm/arm/vgic/vgic-v3.c | 5 -----
>> 1 file changed, 5 deletions(-)
>>
>> diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c
>> index bdcf8e7a6161..72fc688c3e9d 100644
>> --- a/virt/kvm/arm/vgic/vgic-v3.c
>> +++ b/virt/kvm/arm/vgic/vgic-v3.c
>> @@ -552,11 +552,6 @@ int vgic_v3_probe(const struct gic_kvm_info *info)
>> pr_warn("GICV physical address 0x%llx not page aligned\n",
>> (unsigned long long)info->vcpu.start);
>> kvm_vgic_global_state.vcpu_base = 0;
>> - } else if (!PAGE_ALIGNED(resource_size(&info->vcpu))) {
>> - pr_warn("GICV size 0x%llx not a multiple of page size 0x%lx\n",
>> - (unsigned long long)resource_size(&info->vcpu),
>> - PAGE_SIZE);
>> - kvm_vgic_global_state.vcpu_base = 0;
>> } else {
>> kvm_vgic_global_state.vcpu_base = info->vcpu.start;
>> kvm_vgic_global_state.can_emulate_gicv2 = true;
>> --
>> 2.17.0
>>
^ permalink raw reply
* [PATCH RESEND v4 2/2] arm/arm64: KVM: Add KVM_GET/SET_VCPU_EVENTS
From: Christoffer Dall @ 2018-06-09 11:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1528487320-2873-3-git-send-email-gengdongjiu@huawei.com>
On Sat, Jun 09, 2018 at 03:48:40AM +0800, Dongjiu Geng wrote:
> For the migrating VMs, user space may need to know the exception
> state. For example, in the machine A, KVM make an SError pending,
> when migrate to B, KVM also needs to pend an SError.
>
> This new IOCTL exports user-invisible states related to SError.
> Together with appropriate user space changes, user space can get/set
> the SError exception state to do migrate/snapshot/suspend.
>
> Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com>
> ---
> change since v3:
> 1. Fix the memset() issue in the kvm_arm_vcpu_get_events()
>
> change since v2:
> 1. Add kvm_vcpu_events structure definition for arm platform to avoid the build errors.
>
> change since v1:
> Address Marc's comments, thanks Marc's review
> 1. serror_has_esr always true when ARM64_HAS_RAS_EXTN is set
> 2. remove Spurious blank line in kvm_arm_vcpu_set_events()
> 3. rename pend_guest_serror() to kvm_set_sei_esr()
> 4. Make kvm_arm_vcpu_get_events() did all the work rather than having this split responsibility.
> 5. using sizeof(events) instead of sizeof(struct kvm_vcpu_events)
>
> this series patch is separated from https://www.spinics.net/lists/kvm/msg168917.html
> The user space patch is here: https://lists.gnu.org/archive/html/qemu-devel/2018-05/msg06965.html
>
> change since V12:
> 1. change (vcpu->arch.hcr_el2 & HCR_VSE) to !!(vcpu->arch.hcr_el2 & HCR_VSE) in kvm_arm_vcpu_get_events()
>
> Change since V11:
> Address James's comments, thanks James
> 1. Align the struct of kvm_vcpu_events to 64 bytes
> 2. Avoid exposing the stale ESR value in the kvm_arm_vcpu_get_events()
> 3. Change variables 'injected' name to 'serror_pending' in the kvm_arm_vcpu_set_events()
> 4. Change to sizeof(events) from sizeof(struct kvm_vcpu_events) in kvm_arch_vcpu_ioctl()
>
> Change since V10:
> Address James's comments, thanks James
> 1. Merge the helper function with the user.
> 2. Move the ISS_MASK into pend_guest_serror() to clear top bits
> 3. Make kvm_vcpu_events struct align to 4 bytes
> 4. Add something check in the kvm_arm_vcpu_set_events()
> 5. Check kvm_arm_vcpu_get/set_events()'s return value.
> 6. Initialise kvm_vcpu_events to 0 so that padding transferred to user-space doesn't
> contain kernel stack.
> ---
> Documentation/virtual/kvm/api.txt | 31 ++++++++++++++++++++++++++++---
> arch/arm/include/asm/kvm_host.h | 6 ++++++
> arch/arm/include/uapi/asm/kvm.h | 12 ++++++++++++
> arch/arm/kvm/guest.c | 12 ++++++++++++
> arch/arm64/include/asm/kvm_emulate.h | 5 +++++
> arch/arm64/include/asm/kvm_host.h | 7 +++++++
> arch/arm64/include/uapi/asm/kvm.h | 13 +++++++++++++
> arch/arm64/kvm/guest.c | 36 ++++++++++++++++++++++++++++++++++++
> arch/arm64/kvm/inject_fault.c | 6 +++---
> arch/arm64/kvm/reset.c | 1 +
> virt/kvm/arm/arm.c | 19 +++++++++++++++++++
> 11 files changed, 142 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index fdac969..8896737 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -835,11 +835,13 @@ struct kvm_clock_data {
>
> Capability: KVM_CAP_VCPU_EVENTS
> Extended by: KVM_CAP_INTR_SHADOW
> -Architectures: x86
> +Architectures: x86, arm, arm64
> Type: vm ioctl
> Parameters: struct kvm_vcpu_event (out)
> Returns: 0 on success, -1 on error
>
> +X86:
> +
> Gets currently pending exceptions, interrupts, and NMIs as well as related
> states of the vcpu.
>
> @@ -881,15 +883,32 @@ Only two fields are defined in the flags field:
> - KVM_VCPUEVENT_VALID_SMM may be set in the flags field to signal that
> smi contains a valid state.
>
> +ARM, ARM64:
> +
> +Gets currently pending SError exceptions as well as related states of the vcpu.
> +
> +struct kvm_vcpu_events {
> + struct {
> + __u8 serror_pending;
> + __u8 serror_has_esr;
> + /* Align it to 8 bytes */
> + __u8 pad[6];
> + __u64 serror_esr;
> + } exception;
> + __u32 reserved[12];
> +};
> +
> 4.32 KVM_SET_VCPU_EVENTS
>
> -Capability: KVM_CAP_VCPU_EVENTS
> +Capebility: KVM_CAP_VCPU_EVENTS
nit: unintended change?
> Extended by: KVM_CAP_INTR_SHADOW
> -Architectures: x86
> +Architectures: x86, arm, arm64
> Type: vm ioctl
> Parameters: struct kvm_vcpu_event (in)
> Returns: 0 on success, -1 on error
>
> +X86:
> +
> Set pending exceptions, interrupts, and NMIs as well as related states of the
> vcpu.
>
> @@ -910,6 +929,12 @@ shall be written into the VCPU.
>
> KVM_VCPUEVENT_VALID_SMM can only be set if KVM_CAP_X86_SMM is available.
>
> +ARM, ARM64:
> +
> +Set pending SError exceptions as well as related states of the vcpu.
> +
> +See KVM_GET_VCPU_EVENTS for the data structure.
> +
>
> 4.33 KVM_GET_DEBUGREGS
>
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index c7c28c8..39f9901 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -213,6 +213,12 @@ unsigned long kvm_arm_num_regs(struct kvm_vcpu *vcpu);
> int kvm_arm_copy_reg_indices(struct kvm_vcpu *vcpu, u64 __user *indices);
> int kvm_arm_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg);
> int kvm_arm_set_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg);
> +int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
> + struct kvm_vcpu_events *events);
> +
> +int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
> + struct kvm_vcpu_events *events);
> +
> unsigned long kvm_call_hyp(void *hypfn, ...);
> void force_vm_exit(const cpumask_t *mask);
>
> diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h
> index caae484..c3e6975 100644
> --- a/arch/arm/include/uapi/asm/kvm.h
> +++ b/arch/arm/include/uapi/asm/kvm.h
> @@ -124,6 +124,18 @@ struct kvm_sync_regs {
> struct kvm_arch_memory_slot {
> };
>
> +/* for KVM_GET/SET_VCPU_EVENTS */
> +struct kvm_vcpu_events {
> + struct {
> + __u8 serror_pending;
> + __u8 serror_has_esr;
> + /* Align it to 8 bytes */
> + __u8 pad[6];
> + __u64 serror_esr;
> + } exception;
> + __u32 reserved[12];
> +};
> +
> /* If you need to interpret the index values, here is the key: */
> #define KVM_REG_ARM_COPROC_MASK 0x000000000FFF0000
> #define KVM_REG_ARM_COPROC_SHIFT 16
> diff --git a/arch/arm/kvm/guest.c b/arch/arm/kvm/guest.c
> index a18f33e..c685f0e 100644
> --- a/arch/arm/kvm/guest.c
> +++ b/arch/arm/kvm/guest.c
> @@ -261,6 +261,18 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
> return -EINVAL;
> }
>
> +int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
> + struct kvm_vcpu_events *events)
> +{
> + return -EINVAL;
> +}
> +
> +int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
> + struct kvm_vcpu_events *events)
> +{
> + return -EINVAL;
> +}
> +
> int __attribute_const__ kvm_target_cpu(void)
> {
> switch (read_cpuid_part()) {
> diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
> index 1dab3a9..18f61ff 100644
> --- a/arch/arm64/include/asm/kvm_emulate.h
> +++ b/arch/arm64/include/asm/kvm_emulate.h
> @@ -81,6 +81,11 @@ static inline unsigned long *vcpu_hcr(struct kvm_vcpu *vcpu)
> return (unsigned long *)&vcpu->arch.hcr_el2;
> }
>
> +static inline unsigned long vcpu_get_vsesr(struct kvm_vcpu *vcpu)
> +{
> + return vcpu->arch.vsesr_el2;
> +}
> +
> static inline void vcpu_set_vsesr(struct kvm_vcpu *vcpu, u64 vsesr)
> {
> vcpu->arch.vsesr_el2 = vsesr;
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 469de8a..357304a 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -335,6 +335,11 @@ unsigned long kvm_arm_num_regs(struct kvm_vcpu *vcpu);
> int kvm_arm_copy_reg_indices(struct kvm_vcpu *vcpu, u64 __user *indices);
> int kvm_arm_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg);
> int kvm_arm_set_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg);
> +int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
> + struct kvm_vcpu_events *events);
> +
> +int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
> + struct kvm_vcpu_events *events);
>
> #define KVM_ARCH_WANT_MMU_NOTIFIER
> int kvm_unmap_hva(struct kvm *kvm, unsigned long hva);
> @@ -363,6 +368,8 @@ void handle_exit_early(struct kvm_vcpu *vcpu, struct kvm_run *run,
> int kvm_perf_init(void);
> int kvm_perf_teardown(void);
>
> +void kvm_set_sei_esr(struct kvm_vcpu *vcpu, u64 syndrome);
> +
> struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
>
> void __kvm_set_tpidr_el2(u64 tpidr_el2);
> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> index 04b3256..df4faee 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -39,6 +39,7 @@
> #define __KVM_HAVE_GUEST_DEBUG
> #define __KVM_HAVE_IRQ_LINE
> #define __KVM_HAVE_READONLY_MEM
> +#define __KVM_HAVE_VCPU_EVENTS
>
> #define KVM_COALESCED_MMIO_PAGE_OFFSET 1
>
> @@ -153,6 +154,18 @@ struct kvm_sync_regs {
> struct kvm_arch_memory_slot {
> };
>
> +/* for KVM_GET/SET_VCPU_EVENTS */
> +struct kvm_vcpu_events {
> + struct {
> + __u8 serror_pending;
> + __u8 serror_has_esr;
> + /* Align it to 8 bytes */
> + __u8 pad[6];
> + __u64 serror_esr;
> + } exception;
> + __u32 reserved[12];
> +};
> +
> /* If you need to interpret the index values, here is the key: */
> #define KVM_REG_ARM_COPROC_MASK 0x000000000FFF0000
> #define KVM_REG_ARM_COPROC_SHIFT 16
> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> index 56a0260..4426915 100644
> --- a/arch/arm64/kvm/guest.c
> +++ b/arch/arm64/kvm/guest.c
> @@ -289,6 +289,42 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
> return -EINVAL;
> }
>
> +int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
> + struct kvm_vcpu_events *events)
> +{
> + memset(events, 0, sizeof(*events));
> +
> + events->exception.serror_pending = !!(vcpu->arch.hcr_el2 & HCR_VSE);
> + events->exception.serror_has_esr =
> + cpus_have_const_cap(ARM64_HAS_RAS_EXTN);
nit: no need to wrap this line so strangely, just keep it on a single
line (regardless of going slightly over the 80 chars limit).
> +
> + if (events->exception.serror_pending &&
> + events->exception.serror_has_esr)
same here
> + events->exception.serror_esr = vcpu_get_vsesr(vcpu);
> + else
> + events->exception.serror_esr = 0;
> +
> + return 0;
> +}
> +
> +int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
> + struct kvm_vcpu_events *events)
> +{
> + bool serror_pending = events->exception.serror_pending;
> + bool has_esr = events->exception.serror_has_esr;
> +
> + if (serror_pending && has_esr) {
> + if (!cpus_have_const_cap(ARM64_HAS_RAS_EXTN))
> + return -EINVAL;
> +
> + kvm_set_sei_esr(vcpu, events->exception.serror_esr);
> + } else if (serror_pending) {
> + kvm_inject_vabt(vcpu);
> + }
> +
> + return 0;
> +}
> +
> int __attribute_const__ kvm_target_cpu(void)
> {
> unsigned long implementor = read_cpuid_implementor();
> diff --git a/arch/arm64/kvm/inject_fault.c b/arch/arm64/kvm/inject_fault.c
> index d8e7165..a55e91d 100644
> --- a/arch/arm64/kvm/inject_fault.c
> +++ b/arch/arm64/kvm/inject_fault.c
> @@ -164,9 +164,9 @@ void kvm_inject_undefined(struct kvm_vcpu *vcpu)
> inject_undef64(vcpu);
> }
>
> -static void pend_guest_serror(struct kvm_vcpu *vcpu, u64 esr)
> +void kvm_set_sei_esr(struct kvm_vcpu *vcpu, u64 esr)
> {
> - vcpu_set_vsesr(vcpu, esr);
> + vcpu_set_vsesr(vcpu, esr & ESR_ELx_ISS_MASK);
> *vcpu_hcr(vcpu) |= HCR_VSE;
> }
>
> @@ -184,5 +184,5 @@ static void pend_guest_serror(struct kvm_vcpu *vcpu, u64 esr)
> */
> void kvm_inject_vabt(struct kvm_vcpu *vcpu)
> {
> - pend_guest_serror(vcpu, ESR_ELx_ISV);
> + kvm_set_sei_esr(vcpu, ESR_ELx_ISV);
> }
> diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
> index 38c8a64..20e919a 100644
> --- a/arch/arm64/kvm/reset.c
> +++ b/arch/arm64/kvm/reset.c
> @@ -82,6 +82,7 @@ int kvm_arch_dev_ioctl_check_extension(struct kvm *kvm, long ext)
> break;
> case KVM_CAP_SET_GUEST_DEBUG:
> case KVM_CAP_VCPU_ATTRIBUTES:
> + case KVM_CAP_VCPU_EVENTS:
> r = 1;
> break;
> default:
> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> index a4c1b76..79ecba9 100644
> --- a/virt/kvm/arm/arm.c
> +++ b/virt/kvm/arm/arm.c
> @@ -1107,6 +1107,25 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
> r = kvm_arm_vcpu_has_attr(vcpu, &attr);
> break;
> }
> + case KVM_GET_VCPU_EVENTS: {
> + struct kvm_vcpu_events events;
> +
> + if (kvm_arm_vcpu_get_events(vcpu, &events))
> + return -EINVAL;
> +
> + if (copy_to_user(argp, &events, sizeof(events)))
> + return -EFAULT;
> +
> + return 0;
> + }
> + case KVM_SET_VCPU_EVENTS: {
> + struct kvm_vcpu_events events;
> +
> + if (copy_from_user(&events, argp, sizeof(events)))
> + return -EFAULT;
> +
> + return kvm_arm_vcpu_set_events(vcpu, &events);
> + }
> default:
> r = -EINVAL;
> }
> --
> 2.7.4
>
I'll leave it to James to comment on the specifics of the RAS
interaction, but I think the two patches should be re-ordered, so that
the capability patch comes last, after the functionality has been
introduced.
Otherwise this looks reasonable enough.
Thanks,
-Christoffer
^ permalink raw reply
* [PATCH] KVM: arm/arm64: drop resource size check for GICV window
From: Christoffer Dall @ 2018-06-09 11:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1559C9EA-DE8B-4F96-8406-CCBEB0DFD093@linaro.org>
On Sat, Jun 09, 2018 at 12:30:14PM +0200, Ard Biesheuvel wrote:
>
>
> > On 9 Jun 2018, at 12:06, Christoffer Dall <christoffer.dall@arm.com> wrote:
> >
> >> On Fri, Jun 01, 2018 at 05:06:28PM +0200, Ard Biesheuvel wrote:
> >> When booting a 64 KB pages kernel on a ACPI GICv3 system that
> >> implements support for v2 emulation, the following warning is
> >> produced
> >>
> >> GICV size 0x2000 not a multiple of page size 0x10000
> >>
> >> and support for v2 emulation is disabled, preventing GICv2 VMs
> >> from being able to run on such hosts.
> >>
> >> The reason is that vgic_v3_probe() performs a sanity check on the
> >> size of the window (it should be a multiple of the page size),
> >> while the ACPI MADT parsing code hardcodes the size of the window
> >> to 8 KB. This makes sense, considering that ACPI does not bother
> >> to describe the size in the first place, under the assumption that
> >> platforms implementing ACPI will follow the architecture and not
> >> put anything else in the same 64 KB window.
> >
> > Does the architecture actually say that anywhere?
> >
> >>
> >> So let's just drop the sanity check altogether, and assume that
> >> the window is at least 64 KB in size.
> >
> > This could obviously be dangerous if broken systems actually exist.
> > Marc may know more about that than me. An alternative would be to
> > modify the ACPI code to assume max(8 KB, page size) instead, and/or a
> > command line parameter to override this check.
> >
> > That said, I'm not directly opposed to this patch, but I'll let Marc
> > have a look as well.
> >
>
> This approach was actually Marc?s idea, and he already applied the patch to the queue branch afaik.
>
Hmmm, ok.
-Christoffer
^ permalink raw reply
* [PATCH] KVM: arm/arm64: drop resource size check for GICV window
From: Marc Zyngier @ 2018-06-09 12:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180609100657.GI5097@C02W217FHV2R.local>
On Sat, 09 Jun 2018 11:06:57 +0100,
Christoffer Dall wrote:
>
> On Fri, Jun 01, 2018 at 05:06:28PM +0200, Ard Biesheuvel wrote:
> > When booting a 64 KB pages kernel on a ACPI GICv3 system that
> > implements support for v2 emulation, the following warning is
> > produced
> >
> > GICV size 0x2000 not a multiple of page size 0x10000
> >
> > and support for v2 emulation is disabled, preventing GICv2 VMs
> > from being able to run on such hosts.
> >
> > The reason is that vgic_v3_probe() performs a sanity check on the
> > size of the window (it should be a multiple of the page size),
> > while the ACPI MADT parsing code hardcodes the size of the window
> > to 8 KB. This makes sense, considering that ACPI does not bother
> > to describe the size in the first place, under the assumption that
> > platforms implementing ACPI will follow the architecture and not
> > put anything else in the same 64 KB window.
>
> Does the architecture actually say that anywhere?
It implies it in section 8.14 of the GICv3 spec:
<quote>
To enable use of 64KB pages, the GICV_* memory map must ensure that:
* The base address of the GICV_* registers is 64KB aligned.
* An alias of the GICV_* registers is provided starting at offset
0xF000 from the start of the page such that a second copy of
GICV_DIR exists at the start of the next 64KB page. This provides
support for both 4KB and 64KB pages.
</quote>
> > So let's just drop the sanity check altogether, and assume that
> > the window is at least 64 KB in size.
>
> This could obviously be dangerous if broken systems actually exist.
> Marc may know more about that than me. An alternative would be to
> modify the ACPI code to assume max(8 KB, page size) instead, and/or a
> command line parameter to override this check.
While the above is in effect very similar to the corresponding GICv2
requirements with the ARMv8 architecture (described in SBSA, which
everybody and their dog are unfortunately making a point in ignoring),
this is implemented in the CPU, meaning that integrators do not have
the opportunity to fsck it up. Hooray!
And as far as I know, this is only implemented on A35, A53, A57, A72
and A73 (all the other ARMv8 CPUs are purely GICv3, and no other
architectural licensee ever shipped a system with the compat
interface).
> That said, I'm not directly opposed to this patch, but I'll let Marc
> have a look as well.
My take on this is that we should play it as per the architecture, and
only add more checks if we're presented with a non-compliant
implementation.
Thanks,
M.
--
Jazz is not dead, it just smell funny.
^ permalink raw reply
* [PATCH v4 2/3] arm: shmobile: Add the R9A06G032 SMP enabler driver
From: Geert Uytterhoeven @ 2018-06-09 12:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAL_JsqK6Pb5ZbqAATpQNK-C=iVNA655H__5RERMmPTUOrX3Few@mail.gmail.com>
Hi Rob,
On Fri, Jun 8, 2018 at 10:41 PM Rob Herring <robh+dt@kernel.org> wrote:
> On Fri, Jun 8, 2018 at 2:47 AM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Fri, Jun 8, 2018 at 8:50 AM, Michel Pollet
> > <michel.pollet@bp.renesas.com> wrote:
> >>> > + I add this to the cpu.txt, as a separate patch:
> >>> > # On other systems, the property can be either
> >>> > 32 bits or 64 bits, it is the driver's responsibility
> >>> > to deal with either sizes.
> >>>
> >>> That is definitely not what we want to say. Use of 32-bit should be
> >>> considered out of spec. Yes, we have a few platforms in that category, but
> >>> they already handle that themselves. Would be nice to fix them, but at least
> >>> the STi platforms don't seem too active.
> >>>
> >>> IMO, we should delete whatever text we can here and at most just refer to
> >>> the spec.
> >>
> >> So actually I didn't use 32 bits by plain chance, I read the cpu.txt file which says
> >> that 64 bits systems use 64 bits property, concluded that in my case I ought to
> >> use 32 bits, then grepped around and found other systems using 32 bits, therefore
> >> I went forward and used it..
> >>
> >> Nothing said here that it should be 64 bits everywhere -- So the documentation
> >> needs fixing somehow. Right now it certainly led me wrong.
> >
> > Perhaps we should add to Documentation/devicetree/bindings/ the standard
> > bindings from ePAPR and successors, too?
>
> I hope you mean *reference* here, not duplicate the bindings here. We
> want to move in the other direction and move the common bindings out
> of the kernel and into the spec.
I did mean copy... I usually grep in Documentation/devicetree/bindings/,
and fall back to the spec only rarely, mostly because the spec usually
doesn't cover what I need.
I am aware of the ongoing work on updating the spec. I guess it's a
chicken-and-egg problem...
A list of standardized properties under Documentation/devicetree/bindings/,
referring to the spec may be a good interim solution. So at least it would show
it with git grep.
> The real solution here is validation which I'm working on. I had
> already converted cpus.txt. Here's an example of the results of the
> validation:
>
> arch/arm/boot/dts/stih410-b2120.dt.yaml:1962:7: cpu at 0: 'enable-method'
> is a dependency of 'cpu-release-addr'
> arch/arm/boot/dts/stih410-b2120.dt.yaml:1965:26:
> cpu at 0:cpu-release-addr: [155254948] is too short
Thanks, nice!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* [PATCH v2 2/6] arm64: KVM: Handle Set/Way CMOs as NOPs if FWB is present
From: Marc Zyngier @ 2018-06-09 12:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180609092640.GD5097@C02W217FHV2R.local>
On Sat, 09 Jun 2018 10:26:40 +0100,
Christoffer Dall wrote:
>
> On Wed, May 30, 2018 at 01:47:02PM +0100, Marc Zyngier wrote:
> > Set/Way handling is one of the ugliest corners of KVM. We shouldn't
> > have to handle that, but better safe than sorry.
> >
> > Thankfully, FWB fixes this for us by not requiering any maintenance
> > whatsoever, which means we don't have to emulate S/W CMOs, and don't
> > have to track VM ops either.
>
> I tiny bit of rationale here would have been nice. As I understand it,
> if we're presenting the guest with a fully coherent system, there should
> never be a need to invalidate anything, because the guest will always
> see the most recent value no matter how it sings and dances, right?
The guest may not even know about the "fully coherent system". It may
continue to issue its CMOs as before, not realising that they are not
required.
> >
> > We still have to trap S/W though, if only to prevent the guest from
> > doing something bad.
> >
> > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> > ---
> > arch/arm64/kvm/sys_regs.c | 8 +++++++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > index 6e3b969391fd..9a740f159245 100644
> > --- a/arch/arm64/kvm/sys_regs.c
> > +++ b/arch/arm64/kvm/sys_regs.c
> > @@ -195,7 +195,13 @@ static bool access_dcsw(struct kvm_vcpu *vcpu,
> > if (!p->is_write)
> > return read_from_write_only(vcpu, p, r);
> >
> > - kvm_set_way_flush(vcpu);
> > + /*
> > + * Only track S/W ops if we don't have FWB. It still indicates
> > + * that the guest is a bit broken...
> > + */
>
> Is it strictly true that the guest is broken if it does any form of S/W
> ops? Does the guest actually know that it's running on a fully coherent
> system, or is the argument that no software, ever, should do S/W, even
> for reboot etc.?
S/W should really only be used in power-management scenario. I really
cannot think of a single valid (or even safe) reason to issue a S/W
operation outside of PM, when you're guaranteed that there is only a
single CPU up and running. A guest OS cannot enforce this requirement,
so that's really always broken.
> I think this should have slightly more info, or that part of the comment
> should just be dropped, to avoid misleading future readers who don't
> have the full picture.
Happy to add more details when I respin this series.
>
> > + if (!cpus_have_const_cap(ARM64_HAS_STAGE2_FWB))
> > + kvm_set_way_flush(vcpu);
> > +
> > return true;
> > }
> >
> > --
> > 2.17.1
> >
>
> Besides the usual nits on commentary:
>
> Reviewed-by: Christoffer Dall <christoffer.dall@arm.com>
Thanks,
M.
--
Jazz is not dead, it just smell funny.
^ permalink raw reply
* [PATCH v2 4/6] KVM: arm/arm64: Consolidate page-table accessors
From: Marc Zyngier @ 2018-06-09 12:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180609093148.GF5097@C02W217FHV2R.local>
On Sat, 09 Jun 2018 10:31:48 +0100,
Christoffer Dall wrote:
>
> On Wed, May 30, 2018 at 01:47:04PM +0100, Marc Zyngier wrote:
> > The arm and arm64 KVM page tables accessors are pointlessly different
> > between the two architectures, and likely both wrong one way or another:
> > arm64 lacks a dsb(), and arm doesn't use WRITE_ONCE.
> >
> > Let's unify them.
> >
> > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> > ---
> > arch/arm/include/asm/kvm_mmu.h | 12 -----------
> > arch/arm64/include/asm/kvm_mmu.h | 3 ---
> > virt/kvm/arm/mmu.c | 35 ++++++++++++++++++++++++++++----
> > 3 files changed, 31 insertions(+), 19 deletions(-)
> >
> > diff --git a/arch/arm/include/asm/kvm_mmu.h b/arch/arm/include/asm/kvm_mmu.h
> > index 707a1f06dc5d..468ff945efa0 100644
> > --- a/arch/arm/include/asm/kvm_mmu.h
> > +++ b/arch/arm/include/asm/kvm_mmu.h
> > @@ -75,18 +75,6 @@ phys_addr_t kvm_get_idmap_vector(void);
> > int kvm_mmu_init(void);
> > void kvm_clear_hyp_idmap(void);
> >
> > -static inline void kvm_set_pmd(pmd_t *pmd, pmd_t new_pmd)
> > -{
> > - *pmd = new_pmd;
> > - dsb(ishst);
> > -}
> > -
> > -static inline void kvm_set_pte(pte_t *pte, pte_t new_pte)
> > -{
> > - *pte = new_pte;
> > - dsb(ishst);
> > -}
> > -
> > static inline pte_t kvm_s2pte_mkwrite(pte_t pte)
> > {
> > pte_val(pte) |= L_PTE_S2_RDWR;
> > diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
> > index 9dbca5355029..26c89b63f604 100644
> > --- a/arch/arm64/include/asm/kvm_mmu.h
> > +++ b/arch/arm64/include/asm/kvm_mmu.h
> > @@ -170,9 +170,6 @@ phys_addr_t kvm_get_idmap_vector(void);
> > int kvm_mmu_init(void);
> > void kvm_clear_hyp_idmap(void);
> >
> > -#define kvm_set_pte(ptep, pte) set_pte(ptep, pte)
> > -#define kvm_set_pmd(pmdp, pmd) set_pmd(pmdp, pmd)
> > -
> > static inline pte_t kvm_s2pte_mkwrite(pte_t pte)
> > {
> > pte_val(pte) |= PTE_S2_RDWR;
> > diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
> > index ba66bf7ae299..c9ed239c0840 100644
> > --- a/virt/kvm/arm/mmu.c
> > +++ b/virt/kvm/arm/mmu.c
> > @@ -177,6 +177,33 @@ static void clear_stage2_pmd_entry(struct kvm *kvm, pmd_t *pmd, phys_addr_t addr
> > put_page(virt_to_page(pmd));
> > }
> >
> > +static inline void kvm_set_pte(pte_t *ptep, pte_t new_pte)
> > +{
> > + WRITE_ONCE(*ptep, new_pte);
> > + dsb(ishst);
> > +}
> > +
> > +static inline void kvm_set_pmd(pmd_t *pmdp, pmd_t new_pmd)
> > +{
> > + WRITE_ONCE(*pmdp, new_pmd);
> > + dsb(ishst);
> > +}
> > +
>
> arm64 set_pte and set_pmd have an isb() in addition to the dsb(), why
> can we let go of that here?
Good point. There was an offline discussion with Will and Mark a
couple of weeks ago, where we agreed that this ISB wasn't
required. I've of course paged it out. Mark, do you remember the
rational?
Thanks,
M.
--
Jazz is not dead, it just smell funny.
^ permalink raw reply
* [PATCH RESEND v4 2/2] arm/arm64: KVM: Add KVM_GET/SET_VCPU_EVENTS
From: Marc Zyngier @ 2018-06-09 12:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1528487320-2873-3-git-send-email-gengdongjiu@huawei.com>
On Fri, 08 Jun 2018 20:48:40 +0100,
Dongjiu Geng wrote:
>
> For the migrating VMs, user space may need to know the exception
> state. For example, in the machine A, KVM make an SError pending,
> when migrate to B, KVM also needs to pend an SError.
>
> This new IOCTL exports user-invisible states related to SError.
> Together with appropriate user space changes, user space can get/set
> the SError exception state to do migrate/snapshot/suspend.
>
> Signed-off-by: Dongjiu Geng <gengdongjiu@huawei.com>
> ---
> change since v3:
> 1. Fix the memset() issue in the kvm_arm_vcpu_get_events()
>
> change since v2:
> 1. Add kvm_vcpu_events structure definition for arm platform to avoid the build errors.
>
> change since v1:
> Address Marc's comments, thanks Marc's review
> 1. serror_has_esr always true when ARM64_HAS_RAS_EXTN is set
> 2. remove Spurious blank line in kvm_arm_vcpu_set_events()
> 3. rename pend_guest_serror() to kvm_set_sei_esr()
> 4. Make kvm_arm_vcpu_get_events() did all the work rather than having this split responsibility.
> 5. using sizeof(events) instead of sizeof(struct kvm_vcpu_events)
>
> this series patch is separated from https://www.spinics.net/lists/kvm/msg168917.html
> The user space patch is here: https://lists.gnu.org/archive/html/qemu-devel/2018-05/msg06965.html
>
> change since V12:
> 1. change (vcpu->arch.hcr_el2 & HCR_VSE) to !!(vcpu->arch.hcr_el2 & HCR_VSE) in kvm_arm_vcpu_get_events()
>
> Change since V11:
> Address James's comments, thanks James
> 1. Align the struct of kvm_vcpu_events to 64 bytes
> 2. Avoid exposing the stale ESR value in the kvm_arm_vcpu_get_events()
> 3. Change variables 'injected' name to 'serror_pending' in the kvm_arm_vcpu_set_events()
> 4. Change to sizeof(events) from sizeof(struct kvm_vcpu_events) in kvm_arch_vcpu_ioctl()
>
> Change since V10:
> Address James's comments, thanks James
> 1. Merge the helper function with the user.
> 2. Move the ISS_MASK into pend_guest_serror() to clear top bits
> 3. Make kvm_vcpu_events struct align to 4 bytes
> 4. Add something check in the kvm_arm_vcpu_set_events()
> 5. Check kvm_arm_vcpu_get/set_events()'s return value.
> 6. Initialise kvm_vcpu_events to 0 so that padding transferred to user-space doesn't
> contain kernel stack.
> ---
> Documentation/virtual/kvm/api.txt | 31 ++++++++++++++++++++++++++++---
> arch/arm/include/asm/kvm_host.h | 6 ++++++
> arch/arm/include/uapi/asm/kvm.h | 12 ++++++++++++
> arch/arm/kvm/guest.c | 12 ++++++++++++
> arch/arm64/include/asm/kvm_emulate.h | 5 +++++
> arch/arm64/include/asm/kvm_host.h | 7 +++++++
> arch/arm64/include/uapi/asm/kvm.h | 13 +++++++++++++
> arch/arm64/kvm/guest.c | 36 ++++++++++++++++++++++++++++++++++++
> arch/arm64/kvm/inject_fault.c | 6 +++---
> arch/arm64/kvm/reset.c | 1 +
> virt/kvm/arm/arm.c | 19 +++++++++++++++++++
> 11 files changed, 142 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index fdac969..8896737 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -835,11 +835,13 @@ struct kvm_clock_data {
>
> Capability: KVM_CAP_VCPU_EVENTS
> Extended by: KVM_CAP_INTR_SHADOW
> -Architectures: x86
> +Architectures: x86, arm, arm64
> Type: vm ioctl
> Parameters: struct kvm_vcpu_event (out)
> Returns: 0 on success, -1 on error
>
> +X86:
> +
> Gets currently pending exceptions, interrupts, and NMIs as well as related
> states of the vcpu.
>
> @@ -881,15 +883,32 @@ Only two fields are defined in the flags field:
> - KVM_VCPUEVENT_VALID_SMM may be set in the flags field to signal that
> smi contains a valid state.
>
> +ARM, ARM64:
> +
> +Gets currently pending SError exceptions as well as related states of the vcpu.
> +
> +struct kvm_vcpu_events {
> + struct {
> + __u8 serror_pending;
> + __u8 serror_has_esr;
> + /* Align it to 8 bytes */
> + __u8 pad[6];
> + __u64 serror_esr;
> + } exception;
> + __u32 reserved[12];
> +};
> +
> 4.32 KVM_SET_VCPU_EVENTS
>
> -Capability: KVM_CAP_VCPU_EVENTS
> +Capebility: KVM_CAP_VCPU_EVENTS
> Extended by: KVM_CAP_INTR_SHADOW
> -Architectures: x86
> +Architectures: x86, arm, arm64
> Type: vm ioctl
> Parameters: struct kvm_vcpu_event (in)
> Returns: 0 on success, -1 on error
>
> +X86:
> +
> Set pending exceptions, interrupts, and NMIs as well as related states of the
> vcpu.
>
> @@ -910,6 +929,12 @@ shall be written into the VCPU.
>
> KVM_VCPUEVENT_VALID_SMM can only be set if KVM_CAP_X86_SMM is available.
>
> +ARM, ARM64:
> +
> +Set pending SError exceptions as well as related states of the vcpu.
> +
> +See KVM_GET_VCPU_EVENTS for the data structure.
> +
>
> 4.33 KVM_GET_DEBUGREGS
>
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index c7c28c8..39f9901 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -213,6 +213,12 @@ unsigned long kvm_arm_num_regs(struct kvm_vcpu *vcpu);
> int kvm_arm_copy_reg_indices(struct kvm_vcpu *vcpu, u64 __user *indices);
> int kvm_arm_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg);
> int kvm_arm_set_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg);
> +int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
> + struct kvm_vcpu_events *events);
> +
> +int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
> + struct kvm_vcpu_events *events);
> +
> unsigned long kvm_call_hyp(void *hypfn, ...);
> void force_vm_exit(const cpumask_t *mask);
>
> diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h
> index caae484..c3e6975 100644
> --- a/arch/arm/include/uapi/asm/kvm.h
> +++ b/arch/arm/include/uapi/asm/kvm.h
> @@ -124,6 +124,18 @@ struct kvm_sync_regs {
> struct kvm_arch_memory_slot {
> };
>
> +/* for KVM_GET/SET_VCPU_EVENTS */
> +struct kvm_vcpu_events {
> + struct {
> + __u8 serror_pending;
> + __u8 serror_has_esr;
> + /* Align it to 8 bytes */
> + __u8 pad[6];
> + __u64 serror_esr;
> + } exception;
> + __u32 reserved[12];
> +};
> +
> /* If you need to interpret the index values, here is the key: */
> #define KVM_REG_ARM_COPROC_MASK 0x000000000FFF0000
> #define KVM_REG_ARM_COPROC_SHIFT 16
> diff --git a/arch/arm/kvm/guest.c b/arch/arm/kvm/guest.c
> index a18f33e..c685f0e 100644
> --- a/arch/arm/kvm/guest.c
> +++ b/arch/arm/kvm/guest.c
> @@ -261,6 +261,18 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
> return -EINVAL;
> }
>
> +int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
> + struct kvm_vcpu_events *events)
> +{
> + return -EINVAL;
> +}
> +
> +int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
> + struct kvm_vcpu_events *events)
> +{
> + return -EINVAL;
> +}
> +
> int __attribute_const__ kvm_target_cpu(void)
> {
> switch (read_cpuid_part()) {
> diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
> index 1dab3a9..18f61ff 100644
> --- a/arch/arm64/include/asm/kvm_emulate.h
> +++ b/arch/arm64/include/asm/kvm_emulate.h
> @@ -81,6 +81,11 @@ static inline unsigned long *vcpu_hcr(struct kvm_vcpu *vcpu)
> return (unsigned long *)&vcpu->arch.hcr_el2;
> }
>
> +static inline unsigned long vcpu_get_vsesr(struct kvm_vcpu *vcpu)
> +{
> + return vcpu->arch.vsesr_el2;
> +}
> +
> static inline void vcpu_set_vsesr(struct kvm_vcpu *vcpu, u64 vsesr)
> {
> vcpu->arch.vsesr_el2 = vsesr;
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 469de8a..357304a 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -335,6 +335,11 @@ unsigned long kvm_arm_num_regs(struct kvm_vcpu *vcpu);
> int kvm_arm_copy_reg_indices(struct kvm_vcpu *vcpu, u64 __user *indices);
> int kvm_arm_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg);
> int kvm_arm_set_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg);
> +int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
> + struct kvm_vcpu_events *events);
> +
> +int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
> + struct kvm_vcpu_events *events);
>
> #define KVM_ARCH_WANT_MMU_NOTIFIER
> int kvm_unmap_hva(struct kvm *kvm, unsigned long hva);
> @@ -363,6 +368,8 @@ void handle_exit_early(struct kvm_vcpu *vcpu, struct kvm_run *run,
> int kvm_perf_init(void);
> int kvm_perf_teardown(void);
>
> +void kvm_set_sei_esr(struct kvm_vcpu *vcpu, u64 syndrome);
> +
> struct kvm_vcpu *kvm_mpidr_to_vcpu(struct kvm *kvm, unsigned long mpidr);
>
> void __kvm_set_tpidr_el2(u64 tpidr_el2);
> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> index 04b3256..df4faee 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -39,6 +39,7 @@
> #define __KVM_HAVE_GUEST_DEBUG
> #define __KVM_HAVE_IRQ_LINE
> #define __KVM_HAVE_READONLY_MEM
> +#define __KVM_HAVE_VCPU_EVENTS
>
> #define KVM_COALESCED_MMIO_PAGE_OFFSET 1
>
> @@ -153,6 +154,18 @@ struct kvm_sync_regs {
> struct kvm_arch_memory_slot {
> };
>
> +/* for KVM_GET/SET_VCPU_EVENTS */
> +struct kvm_vcpu_events {
> + struct {
> + __u8 serror_pending;
> + __u8 serror_has_esr;
> + /* Align it to 8 bytes */
> + __u8 pad[6];
> + __u64 serror_esr;
> + } exception;
> + __u32 reserved[12];
> +};
> +
> /* If you need to interpret the index values, here is the key: */
> #define KVM_REG_ARM_COPROC_MASK 0x000000000FFF0000
> #define KVM_REG_ARM_COPROC_SHIFT 16
> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> index 56a0260..4426915 100644
> --- a/arch/arm64/kvm/guest.c
> +++ b/arch/arm64/kvm/guest.c
> @@ -289,6 +289,42 @@ int kvm_arch_vcpu_ioctl_set_sregs(struct kvm_vcpu *vcpu,
> return -EINVAL;
> }
>
> +int kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu,
> + struct kvm_vcpu_events *events)
> +{
> + memset(events, 0, sizeof(*events));
> +
> + events->exception.serror_pending = !!(vcpu->arch.hcr_el2 & HCR_VSE);
> + events->exception.serror_has_esr =
> + cpus_have_const_cap(ARM64_HAS_RAS_EXTN);
> +
> + if (events->exception.serror_pending &&
> + events->exception.serror_has_esr)
> + events->exception.serror_esr = vcpu_get_vsesr(vcpu);
> + else
> + events->exception.serror_esr = 0;
Other than the alignment issues that Christoffer already commented on,
you can perfectly remove the "else" clause altogether (we've just
zeroed the whole structure).
> +
> + return 0;
> +}
> +
> +int kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu,
> + struct kvm_vcpu_events *events)
> +{
> + bool serror_pending = events->exception.serror_pending;
> + bool has_esr = events->exception.serror_has_esr;
> +
> + if (serror_pending && has_esr) {
> + if (!cpus_have_const_cap(ARM64_HAS_RAS_EXTN))
> + return -EINVAL;
> +
> + kvm_set_sei_esr(vcpu, events->exception.serror_esr);
> + } else if (serror_pending) {
> + kvm_inject_vabt(vcpu);
> + }
> +
> + return 0;
There was an earlier request to check that all the padding is set to
zero. I still think this makes sense.
Thanks,
M.
--
Jazz is not dead, it just smell funny.
^ permalink raw reply
* [RESEND v2] dmaengine: pxa: add a default requestor policy
From: Robert Jarzmik @ 2018-06-09 12:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180530201249.26972-1-robert.jarzmik@free.fr>
Robert Jarzmik <robert.jarzmik@free.fr> writes:
> As what former drcmr -1 value meant, add a this as a default to each
> channel, ie. that by default no requestor line is used.
>
> This is specifically used for network drivers smc91x and smc911x, and
> needed for their port to slave maps.
>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
> ---
> Since v1: changed -1 to U32_MAX
Hi Vinod,
Could I have your ack on this so that I add this one to the dma slave map serie
after the merge window is closed please ?
Cheers.
--
Robert
> ---
> drivers/dma/pxa_dma.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/dma/pxa_dma.c b/drivers/dma/pxa_dma.c
> index 9505334f9c6e..b31c28b67ad3 100644
> --- a/drivers/dma/pxa_dma.c
> +++ b/drivers/dma/pxa_dma.c
> @@ -762,6 +762,8 @@ static void pxad_free_chan_resources(struct dma_chan *dchan)
> dma_pool_destroy(chan->desc_pool);
> chan->desc_pool = NULL;
>
> + chan->drcmr = U32_MAX;
> + chan->prio = PXAD_PRIO_LOWEST;
> }
>
> static void pxad_free_desc(struct virt_dma_desc *vd)
> @@ -1386,6 +1388,9 @@ static int pxad_init_dmadev(struct platform_device *op,
> c = devm_kzalloc(&op->dev, sizeof(*c), GFP_KERNEL);
> if (!c)
> return -ENOMEM;
> +
> + c->drcmr = U32_MAX;
> + c->prio = PXAD_PRIO_LOWEST;
> c->vc.desc_free = pxad_free_desc;
> vchan_init(&c->vc, &pdev->slave);
> init_waitqueue_head(&c->wq_state);
--
Robert
^ permalink raw reply
* [PATCH v2 05/17] arm64: Add 'ssbd' command-line option
From: Jon Masters @ 2018-06-09 12:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180529121121.24927-6-marc.zyngier@arm.com>
On 05/29/2018 08:11 AM, Marc Zyngier wrote:
> + ssbd= [ARM64,HW]
> + Speculative Store Bypass Disable control
> +
> + On CPUs that are vulnerable to the Speculative
> + Store Bypass vulnerability and offer a
> + firmware based mitigation, this parameter
> + indicates how the mitigation should be used:
> +
> + force-on: Unconditionally enable mitigation for
> + for both kernel and userspace
> + force-off: Unconditionally disable mitigation for
> + for both kernel and userspace
> + kernel: Always enable mitigation in the
> + kernel, and offer a prctl interface
> + to allow userspace to register its
> + interest in being mitigated too.
This should be "spec_store_bypass_disable" and it should have the same
parameters as on x86: "on", "off", "auto". Why not just add "kernel"?
(we had a "kernel" early on for x86 as well, and it might still end up
coming back anyway). If there's a /compelling/ reason to have the Arm
parameter differ, then it should still recognize the x86 parameter,
similarly to how POWER also does that for cross-arch consistency.
We'll add the x86 parameter way of doing it to RHEL anyway.
Jon.
^ permalink raw reply
* [PATCH v2 07/17] arm64: ssbd: Skip apply_ssbd if not using dynamic mitigation
From: Jon Masters @ 2018-06-09 13:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180529121121.24927-8-marc.zyngier@arm.com>
On 05/29/2018 08:11 AM, Marc Zyngier wrote:
> +void __init arm64_enable_wa2_handling(struct alt_instr *alt,
> + __le32 *origptr, __le32 *updptr,
> + int nr_inst)
Where does the name "wa2" come from here?
Jon.
^ permalink raw reply
* [PATCH v2 12/17] arm64: KVM: Add ARCH_WORKAROUND_2 support for guests
From: Jon Masters @ 2018-06-09 13:09 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180529121121.24927-13-marc.zyngier@arm.com>
On 05/29/2018 08:11 AM, Marc Zyngier wrote:
> In order to offer ARCH_WORKAROUND_2 support to guests, we need
> a bit of infrastructure.
>
> Let's add a flag indicating whether or not the guest uses
> SSBD mitigation. Depending on the state of this flag, allow
> KVM to disable ARCH_WORKAROUND_2 before entering the guest,
> and enable it when exiting it.
...Live migration?
Jon.
^ permalink raw reply
* [PATCH v2 00/17] arm64 SSBD (aka Spectre-v4) mitigation
From: Jon Masters @ 2018-06-09 13:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180529121121.24927-1-marc.zyngier@arm.com>
On 05/29/2018 08:11 AM, Marc Zyngier wrote:
> This patch series implements the Linux kernel side of the "Spectre-v4"
> (CVE-2018-3639) mitigation known as "Speculative Store Bypass Disable"
> (SSBD).
Looks good, with the exception of the naming in patch 5, and a question
about how you're handling live migration of VMs (which needs to preserve
mitigation state). Once those are answered I think it's good.
> For all released Arm Cortex-A CPUs that are affected by this issue, then
> the preferred mitigation is simply to set a chicken bit in the firmware
> during CPU initialisation and therefore no change to Linux is required.
> Other CPUs may require the chicken bit to be toggled dynamically (for
> example, when switching between user-mode and kernel-mode) and this is
> achieved by calling into EL3 via an SMC which has been published as part
> of the latest SMCCC specification:
We're asking (server) silicon vendors that can do so inexpensively to
implement both a firmware knob to control the chicken bit and the ATF
interface. This allows some users to disable the mitigation if they want
to, for example in closed lab environments doing CONFIG_BENCHMARKING
comparisons to other arches which might have mitigations disabled. Not
that I like that, but I want Arm to be on an equal footing at least ;)
Jon.
^ permalink raw reply
* [PATCH v2 05/17] arm64: Add 'ssbd' command-line option
From: Marc Zyngier @ 2018-06-09 13:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <e1b89362-ba94-ead9-a930-eac0e1b3ffba@jonmasters.org>
On Sat, 09 Jun 2018 13:53:08 +0100,
Jon Masters wrote:
>
> On 05/29/2018 08:11 AM, Marc Zyngier wrote:
>
> > + ssbd= [ARM64,HW]
> > + Speculative Store Bypass Disable control
> > +
> > + On CPUs that are vulnerable to the Speculative
> > + Store Bypass vulnerability and offer a
> > + firmware based mitigation, this parameter
> > + indicates how the mitigation should be used:
> > +
> > + force-on: Unconditionally enable mitigation for
> > + for both kernel and userspace
> > + force-off: Unconditionally disable mitigation for
> > + for both kernel and userspace
> > + kernel: Always enable mitigation in the
> > + kernel, and offer a prctl interface
> > + to allow userspace to register its
> > + interest in being mitigated too.
>
> This should be "spec_store_bypass_disable" and it should have the same
> parameters as on x86: "on", "off", "auto". Why not just add
> "kernel"?
Feel free to propose a patch that adds the x86 compat option if you
want, but I don't think this option deserves that many letters, and it
is also worth realising the semantics of the mitigation *are*
different. That's the real reason why we have different options.
> (we had a "kernel" early on for x86 as well, and it might still end up
> coming back anyway). If there's a /compelling/ reason to have the Arm
> parameter differ, then it should still recognize the x86 parameter,
> similarly to how POWER also does that for cross-arch consistency.
Well, we should then aim for real consistency (seccomp or not seccomp?
mitigated kernel or not?), and not at the cosmetic level. Once all
arches implement identical behaviours, we'll be in a position to
safely have a common option naming scheme which would encompass the
actual meaning of "on" and "off" (which have opposite meaning between
x86 and arm64).
> We'll add the x86 parameter way of doing it to RHEL anyway.
Great!
M.
--
Jazz is not dead, it just smell funny.
^ permalink raw reply
* [PATCH v2 07/17] arm64: ssbd: Skip apply_ssbd if not using dynamic mitigation
From: Marc Zyngier @ 2018-06-09 13:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <9bd005f5-60e3-510d-24f6-235e98487ced@jonmasters.org>
On Sat, 09 Jun 2018 14:03:51 +0100,
Jon Masters wrote:
>
> On 05/29/2018 08:11 AM, Marc Zyngier wrote:
>
> > +void __init arm64_enable_wa2_handling(struct alt_instr *alt,
> > + __le32 *origptr, __le32 *updptr,
> > + int nr_inst)
>
> Where does the name "wa2" come from here?
ARCH_WORKAROUND_2, as described in the SMCCC specification.
M.
--
Jazz is not dead, it just smell funny.
^ permalink raw reply
* [PATCH v2 12/17] arm64: KVM: Add ARCH_WORKAROUND_2 support for guests
From: Marc Zyngier @ 2018-06-09 13:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <98318e6c-adcd-ddb8-ad92-e7c1fa771c84@jonmasters.org>
On Sat, 09 Jun 2018 14:09:35 +0100,
Jon Masters wrote:
>
> On 05/29/2018 08:11 AM, Marc Zyngier wrote:
> > In order to offer ARCH_WORKAROUND_2 support to guests, we need
> > a bit of infrastructure.
> >
> > Let's add a flag indicating whether or not the guest uses
> > SSBD mitigation. Depending on the state of this flag, allow
> > KVM to disable ARCH_WORKAROUND_2 before entering the guest,
> > and enable it when exiting it.
>
> ...Live migration?
Work in progress.
M.
--
Jazz is not dead, it just smell funny.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox