* Re: [PATCH v5 08/12] KVM: Reinstate gfn_to_pfn_cache with invalidation support
From: David Woodhouse @ 2021-12-09 22:38 UTC (permalink / raw)
To: Paolo Bonzini, kvm
Cc: Anup Patel, wanpengli @ tencent . com, Catalin Marinas,
Joao Martins, Will Deacon, kvmarm, linux-s390,
joro @ 8bytes . org, Huacai Chen, Christian Borntraeger,
Aleksandar Markovic, karahmed, Suzuki K Poulose, butt3rflyh4ck,
Boris Ostrovsky, Alexandru Elisei, linux-arm-kernel,
jmattson @ google . com, seanjc @ google . com,
mtosatti @ redhat . com, linux-mips, James Morse, kvm-riscv,
Marc Zyngier, vkuznets @ redhat . com, linuxppc-dev
In-Reply-To: <6cb2cd57-16f3-d0ec-adf6-cb8fdcbae035@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 509 bytes --]
On Thu, 2021-12-09 at 23:34 +0100, Paolo Bonzini wrote:
> Compared to the review it's missing this hunk:
>
> @@ -265,7 +265,7 @@ void kvm_gfn_to_pfn_cache_unmap(struct kvm *kvm, struct gfn_to_pfn_cache *gpc)
>
> gpc->valid = false;
>
> - old_khva = gpc->khva;
> + old_khva = (void *)((unsigned long)gpc->khva & ~PAGE_MASK);
> old_dirty = gpc->dirty;
> old_gpa = gpc->gpa;
> old_pfn = gpc->pfn;
Ah right, there were two of those. Will fix; thanks.
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5174 bytes --]
^ permalink raw reply
* Re: [PATCH v5 08/12] KVM: Reinstate gfn_to_pfn_cache with invalidation support
From: Paolo Bonzini @ 2021-12-09 22:34 UTC (permalink / raw)
To: David Woodhouse, kvm
Cc: Anup Patel, wanpengli @ tencent . com, Catalin Marinas,
Joao Martins, Will Deacon, kvmarm, linux-s390,
joro @ 8bytes . org, Huacai Chen, Christian Borntraeger,
Aleksandar Markovic, karahmed, Suzuki K Poulose, butt3rflyh4ck,
Boris Ostrovsky, Alexandru Elisei, linux-arm-kernel,
jmattson @ google . com, seanjc @ google . com,
mtosatti @ redhat . com, linux-mips, James Morse, kvm-riscv,
Marc Zyngier, vkuznets @ redhat . com, linuxppc-dev
In-Reply-To: <b614d9ae0fe7910cfa72eee0b4077776f8012e5f.camel@infradead.org>
On 12/9/21 21:40, David Woodhouse wrote:
>
> NP, very useful fixes. Thanks. Incremental patch looks like this. It
> passes the xen_shinfo_test self-test; will test it with real Xen guests
> tomorrow and repost based on your kvm/next tree once it shows up.
Compared to the review it's missing this hunk:
@@ -265,7 +265,7 @@ void kvm_gfn_to_pfn_cache_unmap(struct kvm *kvm, struct gfn_to_pfn_cache *gpc)
gpc->valid = false;
- old_khva = gpc->khva;
+ old_khva = (void *)((unsigned long)gpc->khva & ~PAGE_MASK);
old_dirty = gpc->dirty;
old_gpa = gpc->gpa;
old_pfn = gpc->pfn;
Otherwise looks good, thanks!
Paolo
^ permalink raw reply
* Re: [PATCH v2 2/6] powerpc/kexec_file: Add KEXEC_SIG support.
From: Nayna @ 2021-12-09 21:53 UTC (permalink / raw)
To: Michal Suchánek
Cc: Mimi Zohar, David Howells, keyrings, Paul Mackerras,
Alexander Gordeev, linux-s390, Herbert Xu, Baoquan He,
Christian Borntraeger, James Morris, Lakshmi Ramasubramanian,
Christian Borntraeger, Serge E. Hallyn, Vasily Gorbik,
Rob Herring, Heiko Carstens, linux-crypto, Dmitry Kasatkin,
Hari Bathini, Daniel Axtens, Philipp Rudo, Frank van der Linden,
kexec, linux-kernel, Luis Chamberlain, Sven Schnelle,
linux-security-module, Jessica Yu, linux-integrity, linuxppc-dev,
David S. Miller, Thiago Jung Bauermann, buendgen
In-Reply-To: <20211209092155.GO117207@kunlun.suse.cz>
On 12/9/21 04:21, Michal Suchánek wrote:
> Hello,
Hi,
> On Wed, Dec 08, 2021 at 08:51:47PM -0500, Nayna wrote:
>> On 11/25/21 13:02, Michal Suchanek wrote:
>>> Copy the code from s390x
>>>
>>> Signed-off-by: Michal Suchanek<msuchanek@suse.de>
>>> ---
>>> arch/powerpc/Kconfig | 11 +++++++++++
>>> arch/powerpc/kexec/elf_64.c | 36 ++++++++++++++++++++++++++++++++++++
>>> 2 files changed, 47 insertions(+)
>>>
>>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>>> index ac0c515552fd..ecc1227a77f1 100644
>>> --- a/arch/powerpc/Kconfig
>>> +++ b/arch/powerpc/Kconfig
>>> @@ -561,6 +561,17 @@ config KEXEC_FILE
>>> config ARCH_HAS_KEXEC_PURGATORY
>>> def_bool KEXEC_FILE
>>>
>>> +config KEXEC_SIG
>>> + bool "Verify kernel signature during kexec_file_load() syscall"
>>> + depends on KEXEC_FILE && MODULE_SIG_FORMAT
>> After manually applying the patch, the build is failing with the following
>> error:
>>
>> build failed with error "arch/powerpc/kexec/elf_64.o: In function
>> `elf64_verify_sig':
>> /root/kernel/linus/linux/arch/powerpc/kexec/elf_64.c:160: undefined
>> reference to `verify_appended_signature'"
> This patch does not add call to verify_appended_signature.
>
> Maybe you applied the following patch as well?
Yes, I tried build after applying all the patches.
Thanks & Regards,
- Nayna
^ permalink raw reply
* [Bug 215285] New: power9 le: amdgpu: *ERROR* hw_init of IP block <psp> failed -22
From: bugzilla-daemon @ 2021-12-09 21:41 UTC (permalink / raw)
To: linuxppc-dev
https://bugzilla.kernel.org/show_bug.cgi?id=215285
Bug ID: 215285
Summary: power9 le: amdgpu: *ERROR* hw_init of IP block <psp>
failed -22
Product: Platform Specific/Hardware
Version: 2.5
Kernel Version: 5.15.6
Hardware: PPC-64
OS: Linux
Tree: Mainline
Status: NEW
Severity: high
Priority: P1
Component: PPC-64
Assignee: platform_ppc-64@kernel-bugs.osdl.org
Reporter: sid@aeam.us
Regression: No
POWER9 blackbird system from Talos. GPU is 6600XT. This seems to be a flip flop
regression: https://gitlab.freedesktop.org/drm/amd/-/issues/1519
[ 54.313046] [drm] Found VCN firmware Version ENC: 1.16 DEC: 2 VEP: 0
Revision: 1
[ 54.313054] amdgpu 0000:03:00.0: amdgpu: Will use PSP to load VCN firmware
[ 54.314153] amdgpu 0000:03:00.0: enabling bus mastering
[ 54.570938] [drm:psp_hw_start [amdgpu]] *ERROR* PSP create ring failed!
[ 54.571061] [drm:psp_hw_init [amdgpu]] *ERROR* PSP firmware loading failed
[ 54.571175] [drm:amdgpu_device_fw_loading [amdgpu]] *ERROR* hw_init of IP
block <psp> failed -22
[ 54.571277] amdgpu 0000:03:00.0: amdgpu: amdgpu_device_ip_init failed
[ 54.571279] amdgpu 0000:03:00.0: amdgpu: Fatal error during GPU init
[ 54.571282] amdgpu 0000:03:00.0: amdgpu: amdgpu: finishing device.
[ 54.572789] amdgpu: probe of 0000:03:00.0 failed with error -22
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* Re: [PATCH v5 08/12] KVM: Reinstate gfn_to_pfn_cache with invalidation support
From: David Woodhouse @ 2021-12-09 20:40 UTC (permalink / raw)
To: Paolo Bonzini, kvm
Cc: Anup Patel, wanpengli @ tencent . com, Catalin Marinas,
Joao Martins, Will Deacon, kvmarm, linux-s390,
joro @ 8bytes . org, Huacai Chen, Christian Borntraeger,
Aleksandar Markovic, karahmed, Suzuki K Poulose, butt3rflyh4ck,
Boris Ostrovsky, Alexandru Elisei, linux-arm-kernel,
jmattson @ google . com, seanjc @ google . com,
mtosatti @ redhat . com, linux-mips, James Morse, kvm-riscv,
Marc Zyngier, vkuznets @ redhat . com, linuxppc-dev
In-Reply-To: <b1bacc6f-be56-4108-6e52-4315a021184b@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 3474 bytes --]
On Thu, 2021-12-09 at 19:34 +0100, Paolo Bonzini wrote:
> Sorry for the late review...
NP, very useful fixes. Thanks. Incremental patch looks like this. It
passes the xen_shinfo_test self-test; will test it with real Xen guests
tomorrow and repost based on your kvm/next tree once it shows up.
diff --git a/virt/kvm/pfncache.c b/virt/kvm/pfncache.c
index d8c6e1d4a647..9e3c662f815f 100644
--- a/virt/kvm/pfncache.c
+++ b/virt/kvm/pfncache.c
@@ -124,6 +124,33 @@ static void __release_gpc(struct kvm *kvm, kvm_pfn_t pfn, void *khva,
}
}
+static kvm_pfn_t hva_to_pfn_retry(struct kvm *kvm, unsigned long uhva)
+{
+ unsigned long mmu_seq;
+ kvm_pfn_t new_pfn;
+ int retry;
+
+ do {
+ mmu_seq = kvm->mmu_notifier_seq;
+ smp_rmb();
+
+ /* We always request a writeable mapping */
+ new_pfn = hva_to_pfn(uhva, false, NULL, true, NULL);
+ if (is_error_noslot_pfn(new_pfn))
+ break;
+
+ KVM_MMU_READ_LOCK(kvm);
+ retry = mmu_notifier_retry_hva(kvm, mmu_seq, uhva);
+ KVM_MMU_READ_UNLOCK(kvm);
+ if (!retry)
+ break;
+
+ cond_resched();
+ } while (1);
+
+ return new_pfn;
+}
+
int kvm_gfn_to_pfn_cache_refresh(struct kvm *kvm, struct gfn_to_pfn_cache *gpc,
gpa_t gpa, unsigned long len, bool dirty)
{
@@ -147,7 +174,7 @@ int kvm_gfn_to_pfn_cache_refresh(struct kvm *kvm, struct gfn_to_pfn_cache *gpc,
old_gpa = gpc->gpa;
old_pfn = gpc->pfn;
- old_khva = gpc->khva;
+ old_khva = (void *)((unsigned long)gpc->khva & ~PAGE_MASK);
old_uhva = gpc->uhva;
old_valid = gpc->valid;
old_dirty = gpc->dirty;
@@ -178,8 +205,6 @@ int kvm_gfn_to_pfn_cache_refresh(struct kvm *kvm, struct gfn_to_pfn_cache *gpc,
if (!old_valid || old_uhva != gpc->uhva) {
unsigned long uhva = gpc->uhva;
void *new_khva = NULL;
- unsigned long mmu_seq;
- int retry;
/* Placeholders for "hva is valid but not yet mapped" */
gpc->pfn = KVM_PFN_ERR_FAULT;
@@ -188,28 +213,15 @@ int kvm_gfn_to_pfn_cache_refresh(struct kvm *kvm, struct gfn_to_pfn_cache *gpc,
write_unlock_irq(&gpc->lock);
- retry_map:
- mmu_seq = kvm->mmu_notifier_seq;
- smp_rmb();
-
- /* We always request a writeable mapping */
- new_pfn = hva_to_pfn(uhva, false, NULL, true, NULL);
+ new_pfn = hva_to_pfn_retry(kvm, uhva);
if (is_error_noslot_pfn(new_pfn)) {
ret = -EFAULT;
goto map_done;
}
- KVM_MMU_READ_LOCK(kvm);
- retry = mmu_notifier_retry_hva(kvm, mmu_seq, uhva);
- KVM_MMU_READ_UNLOCK(kvm);
- if (retry) {
- cond_resched();
- goto retry_map;
- }
-
if (gpc->kernel_map) {
if (new_pfn == old_pfn) {
- new_khva = (void *)((unsigned long)old_khva - page_offset);
+ new_khva = old_khva;
old_pfn = KVM_PFN_ERR_FAULT;
old_khva = NULL;
} else if (pfn_valid(new_pfn)) {
@@ -219,7 +231,9 @@ int kvm_gfn_to_pfn_cache_refresh(struct kvm *kvm, struct gfn_to_pfn_cache *gpc,
new_khva = memremap(pfn_to_hpa(new_pfn), PAGE_SIZE, MEMREMAP_WB);
#endif
}
- if (!new_khva)
+ if (new_khva)
+ new_khva += page_offset;
+ else
ret = -EFAULT;
}
@@ -232,7 +246,7 @@ int kvm_gfn_to_pfn_cache_refresh(struct kvm *kvm, struct gfn_to_pfn_cache *gpc,
} else {
/* At this point, gpc->valid may already have been cleared */
gpc->pfn = new_pfn;
- gpc->khva = new_khva + page_offset;
+ gpc->khva = new_khva;
}
} else {
/* If the HVA→PFN mapping was already valid, don't unmap it. */
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5174 bytes --]
^ permalink raw reply related
* [PATCH] PCI/AER: potential dereference of null pointer
From: Jiasheng Jiang @ 2021-12-09 9:45 UTC (permalink / raw)
To: ruscur, oohall, bhelgaas
Cc: linux-pci, Jiasheng Jiang, linuxppc-dev, linux-kernel
he return value of kzalloc() needs to be checked.
To avoid use of null pointer in case of the failure of alloc.
Fixes: db89ccbe52c7 ("PCI/AER: Define aer_stats structure for AER capable devices")
Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
---
drivers/pci/pcie/aer.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index ec943cee5ecc..d04303edf468 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -376,6 +376,8 @@ void pci_aer_init(struct pci_dev *dev)
return;
dev->aer_stats = kzalloc(sizeof(struct aer_stats), GFP_KERNEL);
+ if (!dev->aer_stats)
+ return;
/*
* We save/restore PCI_ERR_UNCOR_MASK, PCI_ERR_UNCOR_SEVER,
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v2 2/6] powerpc/kexec_file: Add KEXEC_SIG support.
From: Nayna @ 2021-12-09 1:51 UTC (permalink / raw)
To: Michal Suchanek, keyrings
Cc: Mimi Zohar, David Howells, Paul Mackerras, Alexander Gordeev,
linux-s390, Herbert Xu, Baoquan He, Christian Borntraeger,
James Morris, Lakshmi Ramasubramanian, Christian Borntraeger,
Serge E. Hallyn, Vasily Gorbik, Rob Herring, Heiko Carstens,
linux-crypto, Dmitry Kasatkin, Hari Bathini, Daniel Axtens,
Philipp Rudo, Frank van der Linden, kexec, linux-kernel,
Luis Chamberlain, Sven Schnelle, linux-security-module,
Jessica Yu, linux-integrity, linuxppc-dev, David S. Miller,
Thiago Jung Bauermann, buendgen
In-Reply-To: <8b30a3c6a4e845eb77f276298424811897efdebf.1637862358.git.msuchanek@suse.de>
[-- Attachment #1: Type: text/plain, Size: 1188 bytes --]
On 11/25/21 13:02, Michal Suchanek wrote:
> Copy the code from s390x
>
> Signed-off-by: Michal Suchanek<msuchanek@suse.de>
> ---
> arch/powerpc/Kconfig | 11 +++++++++++
> arch/powerpc/kexec/elf_64.c | 36 ++++++++++++++++++++++++++++++++++++
> 2 files changed, 47 insertions(+)
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index ac0c515552fd..ecc1227a77f1 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -561,6 +561,17 @@ config KEXEC_FILE
> config ARCH_HAS_KEXEC_PURGATORY
> def_bool KEXEC_FILE
>
> +config KEXEC_SIG
> + bool "Verify kernel signature during kexec_file_load() syscall"
> + depends on KEXEC_FILE && MODULE_SIG_FORMAT
After manually applying the patch, the build is failing with the
following error:
build failed with error "arch/powerpc/kexec/elf_64.o: In function
`elf64_verify_sig':
/root/kernel/linus/linux/arch/powerpc/kexec/elf_64.c:160: undefined
reference to `verify_appended_signature'"
I see it happened because I didn't have MODULE_SIG enabled. Enabling
MODULE_SIG fixes it.
I wonder why not to add "depends on MODULE_SIG" rather than on
MODULE_SIG_FORMAT.
Thanks & Regards,
- Nayna
[-- Attachment #2: Type: text/html, Size: 6227 bytes --]
^ permalink raw reply
* Re: [PATCH 0/4] perf: Add new macros for mem_hops field
From: Arnaldo Carvalho de Melo @ 2021-12-09 19:17 UTC (permalink / raw)
To: Kajol Jain
Cc: mark.rutland, atrajeev, ak, daniel, rnsastry, peterz,
linux-kernel, ast, linux-perf-users, alexander.shishkin, yao.jin,
mingo, paulus, maddy, jolsa, namhyung, songliubraving,
linuxppc-dev, kan.liang
In-Reply-To: <20211206091749.87585-1-kjain@linux.ibm.com>
Em Mon, Dec 06, 2021 at 02:47:45PM +0530, Kajol Jain escreveu:
> Patchset adds new macros for mem_hops field which can be
> used to represent remote-node, socket and board level details.
>
> Currently the code had macro for HOPS_0, which corresponds
> to data coming from another core but same node.
> Add new macros for HOPS_1 to HOPS_3 to represent
> remote-node, socket and board level data.
>
> For ex: Encodings for mem_hops fields with L2 cache:
I checked and this hasn't hit mainstream, is it already merged on a tree
where this is slated to be submitted in the next window? If so please
let me know which one so that I can merge it on perf/core.
- Arnaldo
> L2 - local L2
> L2 | REMOTE | HOPS_0 - remote core, same node L2
> L2 | REMOTE | HOPS_1 - remote node, same socket L2
> L2 | REMOTE | HOPS_2 - remote socket, same board L2
> L2 | REMOTE | HOPS_3 - remote board L2
>
> Patch 1 & 2 adds tool and kernel side changes to add new macros for
> mem_hops field
>
> Patch 3 add data source encodings for power10 and older platforms
> to represent data based on newer composite PERF_MEM_LVLNUM* fields
>
> Patch 4 add data source encodings with proper sub_index used to
> represent memory/cache level data for power10 platform.
>
> Kajol Jain (4):
> perf: Add new macros for mem_hops field
> tools/perf: Add new macros for mem_hops field
> powerpc/perf: Add encodings to represent data based on newer composite
> PERF_MEM_LVLNUM* fields
> powerpc/perf: Add data source encodings for power10 platform
>
> arch/powerpc/perf/isa207-common.c | 60 ++++++++++++++++++++-------
> include/uapi/linux/perf_event.h | 5 ++-
> tools/include/uapi/linux/perf_event.h | 5 ++-
> tools/perf/util/mem-events.c | 29 ++++++++-----
> 4 files changed, 71 insertions(+), 28 deletions(-)
>
> --
> 2.27.0
--
- Arnaldo
^ permalink raw reply
* Re: [PATCH v5 00/12] KVM: x86/xen: Add in-kernel Xen event channel delivery
From: Paolo Bonzini @ 2021-12-09 18:55 UTC (permalink / raw)
To: David Woodhouse, kvm
Cc: Anup Patel, wanpengli @ tencent . com, Catalin Marinas,
Joao Martins, Will Deacon, kvmarm, linux-s390,
joro @ 8bytes . org, Huacai Chen, Christian Borntraeger,
Aleksandar Markovic, karahmed, Suzuki K Poulose, butt3rflyh4ck,
Boris Ostrovsky, Alexandru Elisei, linux-arm-kernel,
jmattson @ google . com, seanjc @ google . com,
mtosatti @ redhat . com, linux-mips, James Morse, kvm-riscv,
Marc Zyngier, vkuznets @ redhat . com, linuxppc-dev
In-Reply-To: <5706a7fec0ffdb18097792374dad90c0400b17cd.camel@infradead.org>
On 12/9/21 19:47, David Woodhouse wrote:
>>> As in the previous two rounds, the last patch (this time patch 12) is
>>> included as illustration of how we*might* use this for fixing the UAF
>>> bugs in nesting, but isn't intended to be applied as-is. Patches 1-11 are.
>> Queued 1-7, will be on kvm/next tomorrow though.
>
> Thanks. I assume you made the changes you wanted to the makefiles then,
> and will work on the gfn_to_pfn_cache changes you suggested.
Yes, thanks.
Paolo
^ permalink raw reply
* Re: [PATCH v5 00/12] KVM: x86/xen: Add in-kernel Xen event channel delivery
From: David Woodhouse @ 2021-12-09 18:47 UTC (permalink / raw)
To: Paolo Bonzini, kvm
Cc: Anup Patel, wanpengli @ tencent . com, Catalin Marinas,
Joao Martins, Will Deacon, kvmarm, linux-s390,
joro @ 8bytes . org, Huacai Chen, Christian Borntraeger,
Aleksandar Markovic, karahmed, Suzuki K Poulose, butt3rflyh4ck,
Boris Ostrovsky, Alexandru Elisei, linux-arm-kernel,
jmattson @ google . com, seanjc @ google . com,
mtosatti @ redhat . com, linux-mips, James Morse, kvm-riscv,
Marc Zyngier, vkuznets @ redhat . com, linuxppc-dev
In-Reply-To: <2617aea0-af09-5c0d-1fd7-65e2a814b516@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 487 bytes --]
On Thu, 2021-12-09 at 19:34 +0100, Paolo Bonzini wrote:
> > As in the previous two rounds, the last patch (this time patch 12) is
> > included as illustration of how we*might* use this for fixing the UAF
> > bugs in nesting, but isn't intended to be applied as-is. Patches 1-11 are.
>
> Queued 1-7, will be on kvm/next tomorrow though.
Thanks. I assume you made the changes you wanted to the makefiles then,
and will work on the gfn_to_pfn_cache changes you suggested.
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5174 bytes --]
^ permalink raw reply
* Re: [PATCH v5 00/12] KVM: x86/xen: Add in-kernel Xen event channel delivery
From: Paolo Bonzini @ 2021-12-09 18:34 UTC (permalink / raw)
To: David Woodhouse, kvm
Cc: Anup Patel, wanpengli @ tencent . com, Catalin Marinas,
Joao Martins, Will Deacon, kvmarm, linux-s390,
joro @ 8bytes . org, Huacai Chen, Christian Borntraeger,
Aleksandar Markovic, karahmed, Suzuki K Poulose, butt3rflyh4ck,
Boris Ostrovsky, Alexandru Elisei, linux-arm-kernel,
jmattson @ google . com, seanjc @ google . com,
mtosatti @ redhat . com, linux-mips, James Morse, kvm-riscv,
Marc Zyngier, vkuznets @ redhat . com, linuxppc-dev
In-Reply-To: <20211121125451.9489-1-dwmw2@infradead.org>
On 11/21/21 13:54, David Woodhouse wrote:
> Introduce the basic concept of 2 level event channels for kernel delivery,
> which is just a simple matter of a few test_and_set_bit calls on a mapped
> shared info page.
>
> This can be used for routing MSI of passthrough devices to PIRQ event
> channels in a Xen guest, and we can build on it for delivering IPIs and
> timers directly from the kernel too.
>
> v1: Use kvm_map_gfn() although I didn't quite see how it works.
>
> v2: Avoid kvm_map_gfn() and implement a safe mapping with invalidation
> support for myself.
>
> v3: Reinvent gfn_to_pfn_cache with sane invalidation semantics, for my
> use case as well as nesting.
>
> v4: Rework dirty handling, as it became apparently that we need an active
> vCPU context to mark pages dirty so it can't be done from the MMU
> notifier duing the invalidation; it has to happen on unmap.
>
> v5: Fix sparse warnings reported by kernel test robot<lkp@intel.com>.
>
> Fix revalidation when memslots change but the resulting HVA stays
> the same. We can use the same kernel mapping in that case, if the
> HVA → PFN translation was valid before. So that probably means we
> shouldn't unmap the "old_hva". Augment the test case to exercise
> that one too.
>
> Include the fix for the dirty ring vs. Xen shinfo oops reported
> by butt3rflyh4ck<butterflyhuangxx@gmail.com>.
>
>
> As in the previous two rounds, the last patch (this time patch 12) is
> included as illustration of how we*might* use this for fixing the UAF
> bugs in nesting, but isn't intended to be applied as-is. Patches 1-11 are.
Queued 1-7, will be on kvm/next tomorrow though.
Paolo
^ permalink raw reply
* Re: [PATCH v5 08/12] KVM: Reinstate gfn_to_pfn_cache with invalidation support
From: Paolo Bonzini @ 2021-12-09 18:34 UTC (permalink / raw)
To: David Woodhouse, kvm
Cc: Anup Patel, wanpengli @ tencent . com, Catalin Marinas,
Joao Martins, Will Deacon, kvmarm, linux-s390,
joro @ 8bytes . org, Huacai Chen, Christian Borntraeger,
Aleksandar Markovic, karahmed, Suzuki K Poulose, butt3rflyh4ck,
Boris Ostrovsky, Alexandru Elisei, linux-arm-kernel,
jmattson @ google . com, seanjc @ google . com,
mtosatti @ redhat . com, linux-mips, James Morse, kvm-riscv,
Marc Zyngier, vkuznets @ redhat . com, linuxppc-dev
In-Reply-To: <20211121125451.9489-9-dwmw2@infradead.org>
Sorry for the late review...
On 11/21/21 13:54, David Woodhouse wrote:
> +EXPORT_SYMBOL_GPL(kvm_gfn_to_pfn_cache_check);
> +
> +static void __release_gpc(struct kvm *kvm, kvm_pfn_t pfn, void *khva,
> + gpa_t gpa, bool dirty)
> +{
> + /* Unmap the old page if it was mapped before, and release it */
> + if (!is_error_noslot_pfn(pfn)) {
> + if (khva) {
> + if (pfn_valid(pfn))
> + kunmap(pfn_to_page(pfn));
> +#ifdef CONFIG_HAS_IOMEM
> + else
> + memunmap(khva);
> +#endif
> + }
Considering that the khva is passed directly to memunmap, perhaps it's
cleaner to ensure it's page-aligned:
diff --git a/virt/kvm/pfncache.c b/virt/kvm/pfncache.c
index 13cae72d39e9..267477bd2972 100644
--- a/virt/kvm/pfncache.c
+++ b/virt/kvm/pfncache.c
@@ -147,7 +147,7 @@ int kvm_gfn_to_pfn_cache_refresh(struct kvm *kvm, struct gfn_to_pfn_cache *gpc,
old_gpa = gpc->gpa;
old_pfn = gpc->pfn;
- old_khva = gpc->khva;
+ old_khva = (void *)((unsigned long)gpc->khva & ~PAGE_MASK);
old_uhva = gpc->uhva;
old_valid = gpc->valid;
old_dirty = gpc->dirty;
@@ -209,7 +209,7 @@ int kvm_gfn_to_pfn_cache_refresh(struct kvm *kvm, struct gfn_to_pfn_cache *gpc,
if (gpc->kernel_map) {
if (new_pfn == old_pfn) {
- new_khva = (void *)((unsigned long)old_khva - page_offset);
+ new_khva = old_khva;
old_pfn = KVM_PFN_ERR_FAULT;
old_khva = NULL;
} else if (pfn_valid(new_pfn)) {
@@ -265,7 +265,7 @@ void kvm_gfn_to_pfn_cache_unmap(struct kvm *kvm, struct gfn_to_pfn_cache *gpc)
gpc->valid = false;
- old_khva = gpc->khva;
+ old_khva = (void *)((unsigned long)gpc->khva & ~PAGE_MASK);
old_dirty = gpc->dirty;
old_gpa = gpc->gpa;
old_pfn = gpc->pfn;
>
> + retry_map:
> + mmu_seq = kvm->mmu_notifier_seq;
> + smp_rmb();
> +
> + /* We always request a writeable mapping */
> + new_pfn = hva_to_pfn(uhva, false, NULL, true, NULL);
> + if (is_error_noslot_pfn(new_pfn)) {
> + ret = -EFAULT;
> + goto map_done;
> + }
> +
> + KVM_MMU_READ_LOCK(kvm);
> + retry = mmu_notifier_retry_hva(kvm, mmu_seq, uhva);
> + KVM_MMU_READ_UNLOCK(kvm);
> + if (retry) {
> + cond_resched();
> + goto retry_map;
> + }
> +
This should also be a separate function, like
static kvm_pfn_t hva_to_pfn_retry(unsigned long uhva)
{
kvm_pfn_t new_pfn
unsigned long mmu_seq;
int retry;
retry_map:
mmu_seq = kvm->mmu_notifier_seq;
smp_rmb();
/* We always request a writeable mapping */
new_pfn = hva_to_pfn(uhva, false, NULL, true, NULL);
if (is_error_noslot_pfn(new_pfn))
return new_pfn;
KVM_MMU_READ_LOCK(kvm);
retry = mmu_notifier_retry_hva(kvm, mmu_seq, uhva);
KVM_MMU_READ_UNLOCK(kvm);
if (retry) {
cond_resched();
goto retry_map;
}
return new_pfn;
}
>
> + write_lock_irq(&gpc->lock);
> + if (ret) {
> + gpc->valid = false;
> + gpc->pfn = KVM_PFN_ERR_FAULT;
> + gpc->khva = NULL;
> + } else {
> + /* At this point, gpc->valid may already have been cleared */
> + gpc->pfn = new_pfn;
> + gpc->khva = new_khva + page_offset;
> + }
Should set gpc->khva only if new_khva != NULL (i.e. only if gpc->kernel_map
is true).
Paolo
^ permalink raw reply related
* Re: [PATCH v5 06/12] KVM: powerpc: Use Makefile.kvm for common files
From: Paolo Bonzini @ 2021-12-09 18:05 UTC (permalink / raw)
To: David Woodhouse, kvm
Cc: Anup Patel, wanpengli @ tencent . com, Catalin Marinas,
Joao Martins, Will Deacon, kvmarm, linux-s390,
joro @ 8bytes . org, Huacai Chen, Christian Borntraeger,
Aleksandar Markovic, karahmed, Suzuki K Poulose, butt3rflyh4ck,
Boris Ostrovsky, Alexandru Elisei, linux-arm-kernel,
jmattson @ google . com, seanjc @ google . com,
mtosatti @ redhat . com, linux-mips, James Morse, kvm-riscv,
Marc Zyngier, vkuznets @ redhat . com, linuxppc-dev
In-Reply-To: <20211121125451.9489-7-dwmw2@infradead.org>
On 11/21/21 13:54, David Woodhouse wrote:
> From: David Woodhouse <dwmw@amazon.co.uk>
>
> It's all fairly baroque but in the end, I don't think there's any reason
> for $(KVM)/irqchip.o to have been handled differently, as they all end
> up in $(kvm-y) in the end anyway, regardless of whether they get there
> via $(common-objs-y) and the CPU-specific object lists.
>
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)
> ---
> arch/powerpc/kvm/Makefile | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/kvm/Makefile b/arch/powerpc/kvm/Makefile
> index 583c14ef596e..245f59118413 100644
> --- a/arch/powerpc/kvm/Makefile
> +++ b/arch/powerpc/kvm/Makefile
> @@ -4,11 +4,8 @@
> #
>
> ccflags-y := -Ivirt/kvm -Iarch/powerpc/kvm
> -KVM := ../../../virt/kvm
>
> -common-objs-y = $(KVM)/kvm_main.o $(KVM)/eventfd.o $(KVM)/binary_stats.o
> -common-objs-$(CONFIG_KVM_VFIO) += $(KVM)/vfio.o
> -common-objs-$(CONFIG_KVM_MMIO) += $(KVM)/coalesced_mmio.o
> +include $(srctree)/virt/kvm/Makefile.kvm
>
> common-objs-y += powerpc.o emulate_loadstore.o
> obj-$(CONFIG_KVM_EXIT_TIMING) += timing.o
> @@ -125,7 +122,6 @@ kvm-book3s_32-objs := \
> kvm-objs-$(CONFIG_KVM_BOOK3S_32) := $(kvm-book3s_32-objs)
>
> kvm-objs-$(CONFIG_KVM_MPIC) += mpic.o
> -kvm-objs-$(CONFIG_HAVE_KVM_IRQ_ROUTING) += $(KVM)/irqchip.o
>
> kvm-objs := $(kvm-objs-m) $(kvm-objs-y)
Same here,
kvm-y += $(kvm-objs-m) $(kvm-objs-y)
would be slightly preferrable IMO.
Paolo
^ permalink raw reply
* Re: [PATCH v5 03/12] KVM: s390: Use Makefile.kvm for common files
From: Paolo Bonzini @ 2021-12-09 17:58 UTC (permalink / raw)
To: David Woodhouse, kvm
Cc: Anup Patel, wanpengli @ tencent . com, Catalin Marinas,
Joao Martins, Will Deacon, kvmarm, linux-s390,
joro @ 8bytes . org, Huacai Chen, Christian Borntraeger,
Aleksandar Markovic, karahmed, Suzuki K Poulose, butt3rflyh4ck,
Boris Ostrovsky, Alexandru Elisei, linux-arm-kernel,
jmattson @ google . com, seanjc @ google . com,
mtosatti @ redhat . com, linux-mips, James Morse, kvm-riscv,
Marc Zyngier, vkuznets @ redhat . com, linuxppc-dev
In-Reply-To: <20211121125451.9489-4-dwmw2@infradead.org>
On 11/21/21 13:54, David Woodhouse wrote:
>
> -kvm-objs := $(common-objs) kvm-s390.o intercept.o interrupt.o priv.o sigp.o
> +kvm-objs := kvm-s390.o intercept.o interrupt.o priv.o sigp.o
> kvm-objs += diag.o gaccess.o guestdbg.o vsie.o pv.o
"kvm-y +=" here (for both lines) would be nicer, it's consistent with
x86 and avoids the question of what happens if you have both kvm-objs
and kvm-y.
Paolo
^ permalink raw reply
* Re: [PATCH] powerpc/603: Fix boot failure with DEBUG_PAGEALLOC and KFENCE
From: Christophe Leroy @ 2021-12-09 17:26 UTC (permalink / raw)
To: Michael Ellerman, mbizon@freebox.fr, Aneesh Kumar K.V
Cc: linuxppc-dev@lists.ozlabs.org, Paul Mackerras,
linux-kernel@vger.kernel.org, stable@vger.kernel.org
In-Reply-To: <8735n2nwcv.fsf@mpe.ellerman.id.au>
Le 09/12/2021 à 07:08, Michael Ellerman a écrit :
> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>> Le 07/12/2021 à 11:34, Maxime Bizon a écrit :
>>>
>>> On Tue, 2021-12-07 at 06:10 +0000, Christophe Leroy wrote:
>>>
>>> Hello,
>>>
>>> With the patch applied and
>>>
>>> CONFIG_DEBUG_PAGEALLOC=y
>>> CONFIG_DEBUG_PAGEALLOC_ENABLE_DEFAULT=y
>>> CONFIG_DEBUG_VM=y
>>>
>>> I get tons of this during boot:
>>>
>>> [ 0.000000] Dentry cache hash table entries: 262144 (order: 8, 1048576 bytes, linear)
>>> [ 0.000000] Inode-cache hash table entries: 131072 (order: 7, 524288 bytes, linear)
>>> [ 0.000000] mem auto-init: stack:off, heap alloc:off, heap free:off
>>> [ 0.000000] ------------[ cut here ]------------
>>> [ 0.000000] WARNING: CPU: 0 PID: 0 at arch/powerpc/mm/pgtable.c:194 set_pte_at+0x18/0x160
>>> [ 0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 5.15.0+ #442
>>> [ 0.000000] NIP: 80015ebc LR: 80016728 CTR: 800166e4
>>> [ 0.000000] REGS: 80751dd0 TRAP: 0700 Not tainted (5.15.0+)
>>> [ 0.000000] MSR: 00021032 <ME,IR,DR,RI> CR: 42228882 XER: 20000000
>>> [ 0.000000]
>>> [ 0.000000] GPR00: 800b8dc8 80751e80 806c6300 807311d8 807a1000 8ffffe84 80751ea8 00000000
>>> [ 0.000000] GPR08: 007a1591 00000001 007a1180 00000000 42224882 00000000 3ff9c608 3fffd79c
>>> [ 0.000000] GPR16: 00000000 00000000 00000000 00000000 00000000 00000000 800166e4 807a2000
>>> [ 0.000000] GPR24: 807a1fff 807311d8 807311d8 807a2000 80768804 00000000 807a1000 007a1180
>>> [ 0.000000] NIP [80015ebc] set_pte_at+0x18/0x160
>>> [ 0.000000] LR [80016728] set_page_attr+0x44/0xc0
>>> [ 0.000000] Call Trace:
>>> [ 0.000000] [80751e80] [80058570] console_unlock+0x340/0x428 (unreliable)
>>> [ 0.000000] [80751ea0] [00000000] 0x0
>>> [ 0.000000] [80751ec0] [800b8dc8] __apply_to_page_range+0x144/0x2a8
>>> [ 0.000000] [80751f00] [80016918] __kernel_map_pages+0x54/0x64
>>> [ 0.000000] [80751f10] [800cfeb0] __free_pages_ok+0x1b0/0x440
>>> [ 0.000000] [80751f50] [805cfc8c] memblock_free_all+0x1d8/0x274
>>> [ 0.000000] [80751f90] [805c5e0c] mem_init+0x3c/0xd0
>>> [ 0.000000] [80751fb0] [805c0bdc] start_kernel+0x404/0x5c4
>>> [ 0.000000] [80751ff0] [000033f0] 0x33f0
>>> [ 0.000000] Instruction dump:
>>> [ 0.000000] 7c630034 83e1000c 5463d97e 7c0803a6 38210010 4e800020 9421ffe0 93e1001c
>>> [ 0.000000] 83e60000 81250000 71290001 41820014 <0fe00000> 7c0802a6 93c10018 90010024
>>>
>>>
>>
>> That's unrelated to this patch.
>>
>> The problem is linked to patch c988cfd38e48 ("powerpc/32: use
>> set_memory_attr()"), which changed from using __set_pte_at() to using
>> set_memory_attr() which uses set_pte_at().
>>
>> set_pte_at() has additional checks and shall not be used to updating an
>> existing PTE.
>>
>> Wondering if I should just use __set_pte_at() instead like in the past,
>> or do like commit 9f7853d7609d ("powerpc/mm: Fix set_memory_*() against
>> concurrent accesses") and use pte_update()
>>
>> Michael, Aneesh, any suggestion ?
>
> The motivation for using pte_update() in that commit is that it does the
> update atomically and also handles flushing the HPTE for 64-bit Hash.
>
> But the books/32 version of pte_update() doesn't do that. In fact
> there's some HPTE handling in __set_pte_at(), but then also a comment
> saying it's handling in a subsequent flush_tlb_xxx().
>
> So that doesn't really help make a decision :)
>
> On the other hand, could you convert those set_memory_attr() calls to
> change_memory_attr() and then eventually drop the former?
Sure, that's probably the best.
Initially I had to implement that set_memory_attr() variant because
change_memory_attr() was doing a pte_clear() that was "sawing off the
branch we're sitting on". In extenso mark_rodata_ro() couldn't use
change_memory_attr() to change the text section to read-only because
mark_rodata_ro() is itself in the text section.
But now that change_memory_attr() is using pte_update() instead of going
via a pte_clear(), it's possible to use it, so that's what I'll do.
Thanks for the idea.
Christophe
^ permalink raw reply
* Re: [PATCH v3] powerpc/pseries: read the lpar name from the firmware
From: Laurent Dufour @ 2021-12-09 15:59 UTC (permalink / raw)
To: Nathan Lynch; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <871r2lopes.fsf@linux.ibm.com>
On 09/12/2021, 14:53:31, Nathan Lynch wrote:
> Laurent Dufour <ldufour@linux.ibm.com> writes:
>> On 08/12/2021, 16:21:29, Nathan Lynch wrote:
>>> Laurent Dufour <ldufour@linux.ibm.com> writes:
>>>> On 07/12/2021, 18:07:50, Nathan Lynch wrote:
>>>>> Laurent Dufour <ldufour@linux.ibm.com> writes:
>>>>>> On 07/12/2021, 15:32:39, Nathan Lynch wrote:
>>>>>>> Is there a reasonable fallback for VMs where this parameter doesn't
>>>>>>> exist? PowerVM partitions should always have it, but what do we want the
>>>>>>> behavior to be on other hypervisors?
>>>>>>
>>>>>> In that case, there is no value displayed in the /proc/powerpc/lparcfg and
>>>>>> the lparstat -i command will fall back to the device tree value. I can't
>>>>>> see any valid reason to report the value defined in the device tree
>>>>>> here.
>>>>>
>>>>> Here's a valid reason :-)
>>>>>
>>>>> lparstat isn't the only possible consumer of the interface, and the
>>>>> 'ibm,partition-name' property and the dynamic system parameter clearly
>>>>> serve a common purpose. 'ibm,partition-name' is provided by qemu.
>>>>
>>>> If the hypervisor is not providing this value, this is not the goal of this
>>>> interface to fetch it from the device tree.
>>>>
>>>> Any consumer should be able to fall back on the device tree value, and
>>>> there is no added value to do such a trick in the kernel when it can be
>>>> done in the user space.
>>>
>>> There is value in imposing a level of abstraction so that the semantics
>>> are:
>>>
>>> * Report the name assigned to the guest by the hosting environment, if
>>> available
>>>
>>> as opposed to
>>>
>>> * Return the string returned by a RTAS call to ibm,get-system-parameter
>>> with token 55, if implemented
>>>
>>> The benefit is that consumers of lparcfg do not have to be coded with
>>> the knowledge that "if a partition_name= line is absent, the
>>> ibm,get-system-parameter RTAS call must have failed, so now I should
>>> read /sys/firmware/devicetree/base/ibm,partition_name." That's the sort
>>> of esoterica that is appropriate for the kernel to encapsulate.
>>>
>>> And I'd say the effort involved (falling back to a root node property
>>> lookup) is proportional to the benefit.
>>>
>>
>> I don't agree.
>> From the kernel point of view, I can't see any benefit, this is adding more
>> complexity to do in the kernel what can be done easily in user space.
>
> Applying this logic, I don't see how adding this to lparcfg would be
> justified at all, because user space can already get at the parameter
> using the privileged rtas syscall. Publish it to unprivileged programs
> over D-Bus or something. That would minimize complexity for the kernel.
As you know, there is a need for unprivileged user to read that value.
Adding this to lparcfg solves this issue. Also, this makes it easy to read
from user space, without the need to get plumbing like D-Bus in the
picture. This is simple, working fine, and user space can easily make the
choice to read the DT value if needed. Please keep in mind that most of the
time the hypervisor is providing that value.
^ permalink raw reply
* Re: [PATCH] powerpc/rtas: Introduce rtas_get_sensor_nonblocking() for pci hotplug driver.
From: Nathan Lynch @ 2021-12-09 15:03 UTC (permalink / raw)
To: mahesh; +Cc: linuxppc-dev, Oliver O'Halloran
In-Reply-To: <20211203134230.iklzvh7fjd6wrzp2@in.ibm.com>
Mahesh J Salgaonkar <mahesh@linux.ibm.com> writes:
> On 2021-11-29 22:53:41 Mon, Nathan Lynch wrote:
>> Mahesh Salgaonkar <mahesh@linux.ibm.com> writes:
>> >
>> > However on certain PHB failures, the rtas call get-sesnor-state() returns
>> > extended busy error (9902) until PHB is recovered by phyp. Once PHB is
>> > recovered, the get-sensor-state() returns success with correct presence
>> > status. The rtas call interface rtas_get_sensor() loops over the rtas call
>> > on extended delay return code (9902) until the return value is either
>> > success (0) or error (-1). This causes the EEH handler to get stuck for ~6
>> > seconds before it could notify that the pci error has been detected and
>> > stop any active operations.
>>
>> I am curious whether you see any difference with "powerpc/rtas:
>> rtas_busy_delay() improvements" which was recently applied. It will
>> cause the the calling task to sleep in response to a 990x status instead
>> of immediately retrying:
>>
>> https://git.kernel.org/powerpc/c/38f7b7067dae0c101be573106018e8af22a90fdf
>>
>> If that commit helps then maybe this change isn't needed.
>
> I tried with above commit but it didn't help.
OK, not too surprising, but thank you for checking.
^ permalink raw reply
* Re: [PATCH v3] PCI hotplug: rpaphp: Error out on busy status from get-sensor-state
From: Nathan Lynch @ 2021-12-09 15:02 UTC (permalink / raw)
To: Mahesh Salgaonkar; +Cc: Tyrel Datwyler, Oliver O'Halloran, linuxppc-dev
In-Reply-To: <163853708110.360679.18375283379078566258.stgit@jupiter>
Mahesh Salgaonkar <mahesh@linux.ibm.com> writes:
> To avoid this issue, fix the pci hotplug driver (rpaphp) to return an error
> if the slot presence state can not be detected immediately. Current
> implementation uses rtas_get_sensor() API which blocks the slot check state
> until rtas call returns success. Change rpaphp_get_sensor_state() to invoke
> rtas_call(get-sensor-state) directly and take actions based on rtas return
> status. This patch now errors out immediately on busy return status from
> rtas_call.
>
> Please note that, only on certain PHB failures, the slot presence check
> returns BUSY condition. In normal cases it returns immediately with a
> correct presence state value. Hence this change has no impact on normal pci
> dlpar operations.
I was wondering about this. This seems to be saying -2/990x cannot
happen in other cases. I couldn't find this specified in the
architecture. It seems a bit risky to me to *always* error out on
-2/990x - won't we have intermittent slot enable failures?
> +/*
> + * RTAS call get-sensor-state(DR_ENTITY_SENSE) return values as per PAPR:
> + * -1: Hardware Error
> + * -2: RTAS_BUSY
> + * -3: Invalid sensor. RTAS Parameter Error.
> + * -9000: Need DR entity to be powered up and unisolated before RTAS call
> + * -9001: Need DR entity to be powered up, but not unisolated, before RTAS call
> + * -9002: DR entity unusable
> + * 990x: Extended delay - where x is a number in the range of 0-5
> + */
> +#define RTAS_HARDWARE_ERROR -1
> +#define RTAS_INVALID_SENSOR -3
> +#define SLOT_UNISOLATED -9000
> +#define SLOT_NOT_UNISOLATED -9001
> +#define SLOT_NOT_USABLE -9002
> +
> +static int rtas_to_errno(int rtas_rc)
> +{
> + int rc;
> +
> + switch (rtas_rc) {
> + case RTAS_HARDWARE_ERROR:
> + rc = -EIO;
> + break;
> + case RTAS_INVALID_SENSOR:
> + rc = -EINVAL;
> + break;
> + case SLOT_UNISOLATED:
> + case SLOT_NOT_UNISOLATED:
> + rc = -EFAULT;
> + break;
> + case SLOT_NOT_USABLE:
> + rc = -ENODEV;
> + break;
> + case RTAS_BUSY:
> + case RTAS_EXTENDED_DELAY_MIN...RTAS_EXTENDED_DELAY_MAX:
> + rc = -EBUSY;
> + break;
> + default:
> + err("%s: unexpected RTAS error %d\n", __func__, rtas_rc);
> + rc = -ERANGE;
> + break;
> + }
> + return rc;
> +}
These conversions look OK to me.
^ permalink raw reply
* Re: [PATCH v2 0/6] KEXEC_SIG with appended signature
From: Michal Suchánek @ 2021-12-09 14:57 UTC (permalink / raw)
To: Nayna
Cc: Mimi Zohar, David Howells, keyrings, Paul Mackerras,
Alexander Gordeev, linux-s390, Herbert Xu, Baoquan He,
Christian Borntraeger, James Morris, Lakshmi Ramasubramanian,
Christian Borntraeger, Serge E. Hallyn, Vasily Gorbik,
Rob Herring, Heiko Carstens, linux-crypto, Dmitry Kasatkin,
Hari Bathini, Daniel Axtens, Philipp Rudo, Frank van der Linden,
kexec, linux-kernel, Luis Chamberlain, Sven Schnelle,
linux-security-module, Jessica Yu, linux-integrity, linuxppc-dev,
David S. Miller, Thiago Jung Bauermann, buendgen
In-Reply-To: <b5e6ec36-a9ec-22f4-be58-28d48bdc38b4@linux.vnet.ibm.com>
Hello,
On Wed, Dec 08, 2021 at 08:50:54PM -0500, Nayna wrote:
>
> On 11/25/21 13:02, Michal Suchanek wrote:
> > Hello,
>
> Hi Michael,
>
> >
> > This is resend of the KEXEC_SIG patchset.
> >
> > The first patch is new because it'a a cleanup that does not require any
> > change to the module verification code.
> >
> > The second patch is the only one that is intended to change any
> > functionality.
> >
> > The rest only deduplicates code but I did not receive any review on that
> > part so I don't know if it's desirable as implemented.
> >
> > The first two patches can be applied separately without the rest.
>
> Patch 2 fails to apply on v5.16-rc4. Can you please also include git
> tree/branch while posting the patches ?
Sorry, I did not have a clean base and the Kconfig had another change.
Here is a tree with the changes applied:
https://github.com/hramrach/kernel/tree/kexec_sig
>
> Secondly, I see that you add the powerpc support in Patch 2 and then modify
> it again in Patch 5 after cleanup. Why not add the support for powerpc after
> the clean up ? This will reduce some rework and also probably simplify
> patches.
That's because I don't know if the later patches will be accepted. By
queueing this patch first it can be applied standalone to ppc tree
without regard for the other patches. It's a copy of the s390 code so it
needs the same rework - not really adding complexity.
Thanks
Michal
^ permalink raw reply
* Re: [PATCH v3] powerpc/pseries: read the lpar name from the firmware
From: Nathan Lynch @ 2021-12-09 13:53 UTC (permalink / raw)
To: Laurent Dufour; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <b6edbf96-4349-c39b-69ee-477b4fdef511@linux.ibm.com>
Laurent Dufour <ldufour@linux.ibm.com> writes:
> On 08/12/2021, 16:21:29, Nathan Lynch wrote:
>> Laurent Dufour <ldufour@linux.ibm.com> writes:
>>> On 07/12/2021, 18:07:50, Nathan Lynch wrote:
>>>> Laurent Dufour <ldufour@linux.ibm.com> writes:
>>>>> On 07/12/2021, 15:32:39, Nathan Lynch wrote:
>>>>>> Is there a reasonable fallback for VMs where this parameter doesn't
>>>>>> exist? PowerVM partitions should always have it, but what do we want the
>>>>>> behavior to be on other hypervisors?
>>>>>
>>>>> In that case, there is no value displayed in the /proc/powerpc/lparcfg and
>>>>> the lparstat -i command will fall back to the device tree value. I can't
>>>>> see any valid reason to report the value defined in the device tree
>>>>> here.
>>>>
>>>> Here's a valid reason :-)
>>>>
>>>> lparstat isn't the only possible consumer of the interface, and the
>>>> 'ibm,partition-name' property and the dynamic system parameter clearly
>>>> serve a common purpose. 'ibm,partition-name' is provided by qemu.
>>>
>>> If the hypervisor is not providing this value, this is not the goal of this
>>> interface to fetch it from the device tree.
>>>
>>> Any consumer should be able to fall back on the device tree value, and
>>> there is no added value to do such a trick in the kernel when it can be
>>> done in the user space.
>>
>> There is value in imposing a level of abstraction so that the semantics
>> are:
>>
>> * Report the name assigned to the guest by the hosting environment, if
>> available
>>
>> as opposed to
>>
>> * Return the string returned by a RTAS call to ibm,get-system-parameter
>> with token 55, if implemented
>>
>> The benefit is that consumers of lparcfg do not have to be coded with
>> the knowledge that "if a partition_name= line is absent, the
>> ibm,get-system-parameter RTAS call must have failed, so now I should
>> read /sys/firmware/devicetree/base/ibm,partition_name." That's the sort
>> of esoterica that is appropriate for the kernel to encapsulate.
>>
>> And I'd say the effort involved (falling back to a root node property
>> lookup) is proportional to the benefit.
>>
>
> I don't agree.
> From the kernel point of view, I can't see any benefit, this is adding more
> complexity to do in the kernel what can be done easily in user space.
Applying this logic, I don't see how adding this to lparcfg would be
justified at all, because user space can already get at the parameter
using the privileged rtas syscall. Publish it to unprivileged programs
over D-Bus or something. That would minimize complexity for the kernel.
^ permalink raw reply
* [PATCH] selftests/powerpc: Add a test of sigreturning to the kernel
From: Michael Ellerman @ 2021-12-09 11:59 UTC (permalink / raw)
To: linuxppc-dev; +Cc: npiggin
We have a general signal fuzzer, sigfuz, which can modify the MSR & NIP
before sigreturn. But the chance of it hitting a kernel address and also
clearing MSR_PR is fairly slim.
So add a specific test of sigreturn to a kernel address, both with and
without attempting to clear MSR_PR (which the kernel must block).
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
.../testing/selftests/powerpc/signal/Makefile | 1 +
.../powerpc/signal/sigreturn_kernel.c | 132 ++++++++++++++++++
2 files changed, 133 insertions(+)
create mode 100644 tools/testing/selftests/powerpc/signal/sigreturn_kernel.c
diff --git a/tools/testing/selftests/powerpc/signal/Makefile b/tools/testing/selftests/powerpc/signal/Makefile
index d6ae54663aed..84e201572466 100644
--- a/tools/testing/selftests/powerpc/signal/Makefile
+++ b/tools/testing/selftests/powerpc/signal/Makefile
@@ -1,5 +1,6 @@
# SPDX-License-Identifier: GPL-2.0
TEST_GEN_PROGS := signal signal_tm sigfuz sigreturn_vdso sig_sc_double_restart
+TEST_GEN_PROGS += sigreturn_kernel
CFLAGS += -maltivec
$(OUTPUT)/signal_tm: CFLAGS += -mhtm
diff --git a/tools/testing/selftests/powerpc/signal/sigreturn_kernel.c b/tools/testing/selftests/powerpc/signal/sigreturn_kernel.c
new file mode 100644
index 000000000000..0a1b6e591eee
--- /dev/null
+++ b/tools/testing/selftests/powerpc/signal/sigreturn_kernel.c
@@ -0,0 +1,132 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Test that we can't sigreturn to kernel addresses, or to kernel mode.
+ */
+
+#define _GNU_SOURCE
+
+#include <stdio.h>
+#include <signal.h>
+#include <stdlib.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+#include "utils.h"
+
+#define MSR_PR (1ul << 14)
+
+static volatile unsigned long long sigreturn_addr;
+static volatile unsigned long long sigreturn_msr_mask;
+
+static void sigusr1_handler(int signo, siginfo_t *si, void *uc_ptr)
+{
+ ucontext_t *uc = (ucontext_t *)uc_ptr;
+
+ if (sigreturn_addr)
+ UCONTEXT_NIA(uc) = sigreturn_addr;
+
+ if (sigreturn_msr_mask)
+ UCONTEXT_MSR(uc) &= sigreturn_msr_mask;
+}
+
+static pid_t fork_child(void)
+{
+ pid_t pid;
+
+ pid = fork();
+ if (pid == 0) {
+ raise(SIGUSR1);
+ exit(0);
+ }
+
+ return pid;
+}
+
+static int expect_segv(pid_t pid)
+{
+ int child_ret;
+
+ waitpid(pid, &child_ret, 0);
+ FAIL_IF(WIFEXITED(child_ret));
+ FAIL_IF(!WIFSIGNALED(child_ret));
+ FAIL_IF(WTERMSIG(child_ret) != 11);
+
+ return 0;
+}
+
+int test_sigreturn_kernel(void)
+{
+ struct sigaction act;
+ int child_ret, i;
+ pid_t pid;
+
+ act.sa_sigaction = sigusr1_handler;
+ act.sa_flags = SA_SIGINFO;
+ sigemptyset(&act.sa_mask);
+
+ FAIL_IF(sigaction(SIGUSR1, &act, NULL));
+
+ for (i = 0; i < 2; i++) {
+ // Return to kernel
+ sigreturn_addr = 0xcull << 60;
+ pid = fork_child();
+ expect_segv(pid);
+
+ // Return to kernel virtual
+ sigreturn_addr = 0xc008ull << 48;
+ pid = fork_child();
+ expect_segv(pid);
+
+ // Return out of range
+ sigreturn_addr = 0xc010ull << 48;
+ pid = fork_child();
+ expect_segv(pid);
+
+ // Return to no-man's land, just below PAGE_OFFSET
+ sigreturn_addr = (0xcull << 60) - (64 * 1024);
+ pid = fork_child();
+ expect_segv(pid);
+
+ // Return to no-man's land, above TASK_SIZE_4PB
+ sigreturn_addr = 0x1ull << 52;
+ pid = fork_child();
+ expect_segv(pid);
+
+ // Return to 0xd space
+ sigreturn_addr = 0xdull << 60;
+ pid = fork_child();
+ expect_segv(pid);
+
+ // Return to 0xe space
+ sigreturn_addr = 0xeull << 60;
+ pid = fork_child();
+ expect_segv(pid);
+
+ // Return to 0xf space
+ sigreturn_addr = 0xfull << 60;
+ pid = fork_child();
+ expect_segv(pid);
+
+ // Attempt to set PR=0 for 2nd loop (should be blocked by kernel)
+ sigreturn_msr_mask = ~MSR_PR;
+ }
+
+ printf("All children killed as expected\n");
+
+ // Don't change address, just MSR, should return to user as normal
+ sigreturn_addr = 0;
+ sigreturn_msr_mask = ~MSR_PR;
+ pid = fork_child();
+ waitpid(pid, &child_ret, 0);
+ FAIL_IF(!WIFEXITED(child_ret));
+ FAIL_IF(WIFSIGNALED(child_ret));
+ FAIL_IF(WEXITSTATUS(child_ret) != 0);
+
+ return 0;
+}
+
+int main(void)
+{
+ return test_harness(test_sigreturn_kernel, "sigreturn_kernel");
+}
--
2.31.1
^ permalink raw reply related
* [Bug 214913] [xfstests generic/051] BUG: Kernel NULL pointer dereference on read at 0x00000108 NIP [c0000000000372e4] tm_cgpr_active+0x14/0x40
From: bugzilla-daemon @ 2021-12-09 11:43 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-214913-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=214913
Michael Ellerman (michael@ellerman.id.au) changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|ASSIGNED |NEEDINFO
--- Comment #5 from Michael Ellerman (michael@ellerman.id.au) ---
Sorry I don't have any idea which commit could have fixed this.
The process that crashed was "fsstress", do you know if it uses io_uring?
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* Re: [PATCH v4 09/10] powerpc/mm: Convert to default topdown mmap layout
From: Christophe Leroy @ 2021-12-09 11:32 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, alex@ghiti.fr,
Benjamin Herrenschmidt, Paul Mackerras
Cc: linux-mm@kvack.org, akpm@linux-foundation.org,
linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
In-Reply-To: <87v8zym39m.fsf@mpe.ellerman.id.au>
Le 09/12/2021 à 12:22, Michael Ellerman a écrit :
> Nicholas Piggin <npiggin@gmail.com> writes:
>
>> Excerpts from Christophe Leroy's message of December 9, 2021 8:22 pm:
>>>
>>>
>>> Le 09/12/2021 à 11:15, Nicholas Piggin a écrit :
>>>> Excerpts from Christophe Leroy's message of December 9, 2021 3:18 am:
>>>>> Select CONFIG_ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT and
>>>>> remove arch/powerpc/mm/mmap.c
>>>>>
>>>>> This change provides standard randomisation of mmaps.
>>>>>
>>>>> See commit 8b8addf891de ("x86/mm/32: Enable full randomization on i386
>>>>> and X86_32") for all the benefits of mmap randomisation.
>>>>
>>>> The justification seems pretty reasonable.
>>>>
>>>>>
>>>>> Comparison between powerpc implementation and the generic one:
>>>>> - mmap_is_legacy() is identical.
>>>>> - arch_mmap_rnd() does exactly the same allthough it's written
>>>>> slightly differently.
>>>>> - MIN_GAP and MAX_GAP are identical.
>>>>> - mmap_base() does the same but uses STACK_RND_MASK which provides
>>>>> the same values as stack_maxrandom_size().
>>>>> - arch_pick_mmap_layout() is almost identical. The only difference
>>>>> is that it also adds the random factor to mm->mmap_base in legacy mode.
>>>>>
>>>>> That last point is what provides the standard randomisation of mmaps.
>>>>
>>>> Thanks for describing it. Could you add random_factor to mmap_base for
>>>> the legacy path for powerpc as a 2-line change that adds the legacy
>>>> randomisation. And then this bigger patch would be closer to a no-op.
>>>>
>>>
>>> You mean you would like to see the following patch before doing the
>>> convert ?
>>>
>>> https://patchwork.ozlabs.org/project/linuxppc-dev/patch/7dabf1cbde67a346a187881d4f0bd17347e0334a.1533732583.git.christophe.leroy@c-s.fr/
>>
>> Yes.
>
> My comment at the time was:
>
> Basically mmap_is_legacy() tells you if any of these is true:
>
> - process has the ADDR_COMPAT_LAYOUT personality
> - global legacy_va_layout sysctl is enabled
> - stack is unlimited
>
> And we only want to change the behaviour for the stack. Or at least the
> change log of your patch only talks about the stack limit, not the
> others.
>
> Possibly we should just enable randomisation for all three of those
> cases, but if so we must spell it out in the patch.
>
> It'd also be good to see the output of /proc/x/maps for some processes
> before and after, to show what actually changes.
>
>
> From: https://github.com/linuxppc/issues/issues/59#issuecomment-502066947
>
>
> So I think at least the change log on that patch still needs updating to
> be clear that it's changing behaviour for all mmap_is_legacy() cases,
> not just the stack unlimited case.
>
> There's also a risk changing the mmap legacy behaviour breaks something.
> But we are at least matching the behaviour of other architectures, and
> there is also an escape hatch in the form of `setarch -R`.
>
That was the purpose of adding in the change log a reference to commit
8b8addf891de ("x86/mm/32: Enable full randomization on i386
and X86_32")
All this applies to powerpc as well.
But I can copy paste the changelog of that commit into mine if you think
it is more explicit.
I agree that old patch was only refering to stack limit, I had no clue
of everything else at that time.
Christophe
^ permalink raw reply
* Re: [PATCH v4 09/10] powerpc/mm: Convert to default topdown mmap layout
From: Michael Ellerman @ 2021-12-09 11:22 UTC (permalink / raw)
To: Nicholas Piggin, alex@ghiti.fr, Benjamin Herrenschmidt,
Christophe Leroy, Paul Mackerras
Cc: linux-mm@kvack.org, akpm@linux-foundation.org,
linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
In-Reply-To: <1639046542.qkwu4mjtew.astroid@bobo.none>
Nicholas Piggin <npiggin@gmail.com> writes:
> Excerpts from Christophe Leroy's message of December 9, 2021 8:22 pm:
>>
>>
>> Le 09/12/2021 à 11:15, Nicholas Piggin a écrit :
>>> Excerpts from Christophe Leroy's message of December 9, 2021 3:18 am:
>>>> Select CONFIG_ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT and
>>>> remove arch/powerpc/mm/mmap.c
>>>>
>>>> This change provides standard randomisation of mmaps.
>>>>
>>>> See commit 8b8addf891de ("x86/mm/32: Enable full randomization on i386
>>>> and X86_32") for all the benefits of mmap randomisation.
>>>
>>> The justification seems pretty reasonable.
>>>
>>>>
>>>> Comparison between powerpc implementation and the generic one:
>>>> - mmap_is_legacy() is identical.
>>>> - arch_mmap_rnd() does exactly the same allthough it's written
>>>> slightly differently.
>>>> - MIN_GAP and MAX_GAP are identical.
>>>> - mmap_base() does the same but uses STACK_RND_MASK which provides
>>>> the same values as stack_maxrandom_size().
>>>> - arch_pick_mmap_layout() is almost identical. The only difference
>>>> is that it also adds the random factor to mm->mmap_base in legacy mode.
>>>>
>>>> That last point is what provides the standard randomisation of mmaps.
>>>
>>> Thanks for describing it. Could you add random_factor to mmap_base for
>>> the legacy path for powerpc as a 2-line change that adds the legacy
>>> randomisation. And then this bigger patch would be closer to a no-op.
>>>
>>
>> You mean you would like to see the following patch before doing the
>> convert ?
>>
>> https://patchwork.ozlabs.org/project/linuxppc-dev/patch/7dabf1cbde67a346a187881d4f0bd17347e0334a.1533732583.git.christophe.leroy@c-s.fr/
>
> Yes.
My comment at the time was:
Basically mmap_is_legacy() tells you if any of these is true:
- process has the ADDR_COMPAT_LAYOUT personality
- global legacy_va_layout sysctl is enabled
- stack is unlimited
And we only want to change the behaviour for the stack. Or at least the
change log of your patch only talks about the stack limit, not the
others.
Possibly we should just enable randomisation for all three of those
cases, but if so we must spell it out in the patch.
It'd also be good to see the output of /proc/x/maps for some processes
before and after, to show what actually changes.
From: https://github.com/linuxppc/issues/issues/59#issuecomment-502066947
So I think at least the change log on that patch still needs updating to
be clear that it's changing behaviour for all mmap_is_legacy() cases,
not just the stack unlimited case.
There's also a risk changing the mmap legacy behaviour breaks something.
But we are at least matching the behaviour of other architectures, and
there is also an escape hatch in the form of `setarch -R`.
cheers
^ permalink raw reply
* [Bug 215217] Kernel fails to boot at an early stage when built with GCC_PLUGIN_LATENT_ENTROPY=y (PowerMac G4 3,6)
From: bugzilla-daemon @ 2021-12-09 11:22 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-215217-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=215217
--- Comment #8 from Christophe Leroy (christophe.leroy@csgroup.eu) ---
early_32.o should likely also have DISABLE_LATENT_ENTROPY_PLUGIN, maybe even
more important that for setup_32.o
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
^ 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