* Re: [PATCH kernel 2/2] KVM: PPC: Allow backing bigger guest IOMMU pages with smaller physical pages
From: Balbir Singh @ 2018-05-02 5:53 UTC (permalink / raw)
To: Alexey Kardashevskiy; +Cc: linuxppc-dev, kvm-ppc, David Gibson
In-Reply-To: <20180502040723.20545-3-aik@ozlabs.ru>
On Wed, 2 May 2018 14:07:23 +1000
Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> At the moment we only support in the host the IOMMU page sizes which
> the guest is aware of, which is 4KB/64KB/16MB. However P9 does not support
> 16MB IOMMU pages, 2MB and 1GB pages are supported instead. We can still
> emulate bigger guest pages (for example 16MB) with smaller host pages
> (4KB/64KB/2MB).
>
> This allows the physical IOMMU pages to use a page size smaller or equal
> than the guest visible IOMMU page size.
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> arch/powerpc/kvm/book3s_64_vio.c | 66 +++++++++++++++++++++++++++++--------
> arch/powerpc/kvm/book3s_64_vio_hv.c | 52 +++++++++++++++++++++++++----
> 2 files changed, 98 insertions(+), 20 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_64_vio.c b/arch/powerpc/kvm/book3s_64_vio.c
> index 041e54d..e10d6a3 100644
> --- a/arch/powerpc/kvm/book3s_64_vio.c
> +++ b/arch/powerpc/kvm/book3s_64_vio.c
> @@ -176,14 +176,12 @@ extern long kvm_spapr_tce_attach_iommu_group(struct kvm *kvm, int tablefd,
>
> if (!tbltmp)
> continue;
> - /*
> - * Make sure hardware table parameters are exactly the same;
> - * this is used in the TCE handlers where boundary checks
> - * use only the first attached table.
> - */
> - if ((tbltmp->it_page_shift == stt->page_shift) &&
> - (tbltmp->it_offset == stt->offset) &&
> - (tbltmp->it_size == stt->size)) {
> + /* Make sure hardware table parameters are compatible */
> + if ((tbltmp->it_page_shift <= stt->page_shift) &&
> + (tbltmp->it_offset << tbltmp->it_page_shift ==
> + stt->offset << stt->page_shift) &&
> + (tbltmp->it_size << tbltmp->it_page_shift ==
> + stt->size << stt->page_shift)) {
Very difficult to parse
How about -
subpages_shift = (stt->page_shift - tbl->it_page_shift)
and then
matches_offset = !(tbltmp->it_offset - (stt->it_offset << subpages_shift));
matches_size = !(tbltmp->it_size - (stt->it_size << subpages_shift));
The condition then is just
if ((tbltmp->it_page_shift == stt->page_shift) &&
matches_offset && matches_size) {
> /*
> * Reference the table to avoid races with
> * add/remove DMA windows.
> @@ -396,7 +394,7 @@ static long kvmppc_tce_iommu_mapped_dec(struct kvm *kvm,
> return H_SUCCESS;
> }
>
> -static long kvmppc_tce_iommu_unmap(struct kvm *kvm,
> +static long kvmppc_tce_iommu_do_unmap(struct kvm *kvm,
> struct iommu_table *tbl, unsigned long entry)
> {
> enum dma_data_direction dir = DMA_NONE;
> @@ -416,7 +414,25 @@ static long kvmppc_tce_iommu_unmap(struct kvm *kvm,
> return ret;
> }
>
> -long kvmppc_tce_iommu_map(struct kvm *kvm, struct iommu_table *tbl,
> +static long kvmppc_tce_iommu_unmap(struct kvm *kvm,
> + struct kvmppc_spapr_tce_table *stt, struct iommu_table *tbl,
> + unsigned long entry)
> +{
> + unsigned long ret = H_SUCCESS;
> + unsigned long subpages = 1ULL << (stt->page_shift - tbl->it_page_shift);
> + unsigned long io_entry = entry * subpages;
How do we know this multiplication does not overflow?
> + unsigned long subpg;
Why not just i?
> +
> + for (subpg = 0; subpg < subpages; ++subpg) {
> + ret = kvmppc_tce_iommu_do_unmap(kvm, tbl, io_entry + subpg);
> + if (ret != H_SUCCESS)
> + break;
> + }
> +
> + return ret;
> +}
> +
> +long kvmppc_tce_iommu_do_map(struct kvm *kvm, struct iommu_table *tbl,
> unsigned long entry, unsigned long ua,
> enum dma_data_direction dir)
> {
> @@ -453,6 +469,28 @@ long kvmppc_tce_iommu_map(struct kvm *kvm, struct iommu_table *tbl,
> return 0;
> }
>
> +static long kvmppc_tce_iommu_map(struct kvm *kvm,
> + struct kvmppc_spapr_tce_table *stt, struct iommu_table *tbl,
> + unsigned long entry, unsigned long ua,
> + enum dma_data_direction dir)
> +{
> + unsigned long ret = H_SUCCESS;
> + unsigned long subpages = 1ULL << (stt->page_shift - tbl->it_page_shift);
> + unsigned long io_entry = entry * subpages;
> + unsigned long subpg, pgoff;
> +
> + for (subpg = 0, pgoff = 0; subpg < subpages;
> + ++subpg, pgoff += IOMMU_PAGE_SIZE(tbl)) {
> +
> + ret = kvmppc_tce_iommu_do_map(kvm, tbl,
> + io_entry + subpg, ua + pgoff, dir);
> + if (ret != H_SUCCESS)
> + break;
> + }
> +
> + return ret;
> +}
> +
> long kvmppc_h_put_tce(struct kvm_vcpu *vcpu, unsigned long liobn,
> unsigned long ioba, unsigned long tce)
> {
> @@ -491,10 +529,10 @@ long kvmppc_h_put_tce(struct kvm_vcpu *vcpu, unsigned long liobn,
>
> list_for_each_entry_lockless(stit, &stt->iommu_tables, next) {
> if (dir == DMA_NONE)
> - ret = kvmppc_tce_iommu_unmap(vcpu->kvm,
> + ret = kvmppc_tce_iommu_unmap(vcpu->kvm, stt,
> stit->tbl, entry);
> else
> - ret = kvmppc_tce_iommu_map(vcpu->kvm, stit->tbl,
> + ret = kvmppc_tce_iommu_map(vcpu->kvm, stt, stit->tbl,
> entry, ua, dir);
>
> if (ret == H_SUCCESS)
> @@ -570,7 +608,7 @@ long kvmppc_h_put_tce_indirect(struct kvm_vcpu *vcpu,
> return H_PARAMETER;
>
> list_for_each_entry_lockless(stit, &stt->iommu_tables, next) {
> - ret = kvmppc_tce_iommu_map(vcpu->kvm,
> + ret = kvmppc_tce_iommu_map(vcpu->kvm, stt,
> stit->tbl, entry + i, ua,
> iommu_tce_direction(tce));
>
> @@ -618,7 +656,7 @@ long kvmppc_h_stuff_tce(struct kvm_vcpu *vcpu,
> unsigned long entry = ioba >> stt->page_shift;
>
> for (i = 0; i < npages; ++i) {
> - ret = kvmppc_tce_iommu_unmap(vcpu->kvm,
> + ret = kvmppc_tce_iommu_unmap(vcpu->kvm, stt,
> stit->tbl, entry + i);
>
> if (ret == H_SUCCESS)
> diff --git a/arch/powerpc/kvm/book3s_64_vio_hv.c b/arch/powerpc/kvm/book3s_64_vio_hv.c
> index e220fab..258e786 100644
> --- a/arch/powerpc/kvm/book3s_64_vio_hv.c
> +++ b/arch/powerpc/kvm/book3s_64_vio_hv.c
> @@ -221,7 +221,7 @@ static long kvmppc_rm_tce_iommu_mapped_dec(struct kvm *kvm,
> return H_SUCCESS;
> }
>
> -static long kvmppc_rm_tce_iommu_unmap(struct kvm *kvm,
> +static long kvmppc_rm_tce_iommu_do_unmap(struct kvm *kvm,
> struct iommu_table *tbl, unsigned long entry)
> {
> enum dma_data_direction dir = DMA_NONE;
> @@ -245,7 +245,25 @@ static long kvmppc_rm_tce_iommu_unmap(struct kvm *kvm,
> return ret;
> }
>
> -static long kvmppc_rm_tce_iommu_map(struct kvm *kvm, struct iommu_table *tbl,
> +static long kvmppc_rm_tce_iommu_unmap(struct kvm *kvm,
> + struct kvmppc_spapr_tce_table *stt, struct iommu_table *tbl,
> + unsigned long entry)
> +{
> + unsigned long ret = H_SUCCESS;
> + unsigned long subpages = 1ULL << (stt->page_shift - tbl->it_page_shift);
> + unsigned long io_entry = entry * subpages;
> + unsigned long subpg;
> +
> + for (subpg = 0; subpg < subpages; ++subpg) {
> + ret = kvmppc_rm_tce_iommu_do_unmap(kvm, tbl, io_entry + subpg);
> + if (ret != H_SUCCESS)
> + break;
> + }
> +
> + return ret;
> +}
> +
> +static long kvmppc_rm_tce_iommu_do_map(struct kvm *kvm, struct iommu_table *tbl,
> unsigned long entry, unsigned long ua,
> enum dma_data_direction dir)
> {
> @@ -290,6 +308,28 @@ static long kvmppc_rm_tce_iommu_map(struct kvm *kvm, struct iommu_table *tbl,
> return 0;
> }
>
> +static long kvmppc_rm_tce_iommu_map(struct kvm *kvm,
> + struct kvmppc_spapr_tce_table *stt, struct iommu_table *tbl,
> + unsigned long entry, unsigned long ua,
> + enum dma_data_direction dir)
> +{
> + unsigned long ret = H_SUCCESS;
> + unsigned long subpages = 1ULL << (stt->page_shift - tbl->it_page_shift);
> + unsigned long io_entry = entry * subpages;
> + unsigned long subpg, pgoff;
> +
> + for (subpg = 0, pgoff = 0; subpg < subpages;
> + ++subpg, pgoff += IOMMU_PAGE_SIZE(tbl)) {
> +
> + ret = kvmppc_rm_tce_iommu_do_map(kvm, tbl,
> + io_entry + subpg, ua + pgoff, dir);
> + if (ret != H_SUCCESS)
> + break;
> + }
> +
> + return ret;
> +}
> +
> long kvmppc_rm_h_put_tce(struct kvm_vcpu *vcpu, unsigned long liobn,
> unsigned long ioba, unsigned long tce)
> {
> @@ -327,10 +367,10 @@ long kvmppc_rm_h_put_tce(struct kvm_vcpu *vcpu, unsigned long liobn,
>
> list_for_each_entry_lockless(stit, &stt->iommu_tables, next) {
> if (dir == DMA_NONE)
> - ret = kvmppc_rm_tce_iommu_unmap(vcpu->kvm,
> + ret = kvmppc_rm_tce_iommu_unmap(vcpu->kvm, stt,
> stit->tbl, entry);
> else
> - ret = kvmppc_rm_tce_iommu_map(vcpu->kvm,
> + ret = kvmppc_rm_tce_iommu_map(vcpu->kvm, stt,
> stit->tbl, entry, ua, dir);
>
> if (ret == H_SUCCESS)
> @@ -477,7 +517,7 @@ long kvmppc_rm_h_put_tce_indirect(struct kvm_vcpu *vcpu,
> return H_PARAMETER;
>
> list_for_each_entry_lockless(stit, &stt->iommu_tables, next) {
> - ret = kvmppc_rm_tce_iommu_map(vcpu->kvm,
> + ret = kvmppc_rm_tce_iommu_map(vcpu->kvm, stt,
> stit->tbl, entry + i, ua,
> iommu_tce_direction(tce));
>
> @@ -529,7 +569,7 @@ long kvmppc_rm_h_stuff_tce(struct kvm_vcpu *vcpu,
> unsigned long entry = ioba >> stt->page_shift;
>
> for (i = 0; i < npages; ++i) {
> - ret = kvmppc_rm_tce_iommu_unmap(vcpu->kvm,
> + ret = kvmppc_rm_tce_iommu_unmap(vcpu->kvm, stt,
> stit->tbl, entry + i);
>
> if (ret == H_SUCCESS)
^ permalink raw reply
* Re: [resend] Revert "powerpc/powernv: Increase memory block size to 1GB on radix"
From: rashmica @ 2018-05-02 5:10 UTC (permalink / raw)
To: Balbir Singh, linuxppc-dev; +Cc: Michael Neuling
In-Reply-To: <20180501025725.7441-1-bsingharora@gmail.com>
Tested hot-unplugging dimm device on radix guest on p9 host with KVM.
On 01/05/18 12:57, Balbir Singh wrote:
> This commit was a stop-gap to prevent crashes on hotunplug, caused by
> the mismatch between the 1G mappings used for the linear mapping and the
> memory block size. Those issues are now resolved because we split the
> linear mapping at hotunplug time if necessary, as implemented in commit
> 4dd5f8a99e79 ("powerpc/mm/radix: Split linear mapping on hot-unplug").
>
> Signed-off-by: Balbir Singh <bsingharora@gmail.com>
> Signed-off-by: Michael Neuling <mikey@neuling.org>
Tested-by: Rashmica Gupta <rashmica.g@gmail.com>
> ---
>
> Resend with a newer commit message grabbed from an email sent by mpe.
>
> arch/powerpc/platforms/powernv/setup.c | 10 +---------
> 1 file changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/setup.c b/arch/powerpc/platforms/powernv/setup.c
> index ef8c9ce53a61..fa63d3fff14c 100644
> --- a/arch/powerpc/platforms/powernv/setup.c
> +++ b/arch/powerpc/platforms/powernv/setup.c
> @@ -356,15 +356,7 @@ static void pnv_kexec_cpu_down(int crash_shutdown, int secondary)
> #ifdef CONFIG_MEMORY_HOTPLUG_SPARSE
> static unsigned long pnv_memory_block_size(void)
> {
> - /*
> - * We map the kernel linear region with 1GB large pages on radix. For
> - * memory hot unplug to work our memory block size must be at least
> - * this size.
> - */
> - if (radix_enabled())
> - return 1UL * 1024 * 1024 * 1024;
> - else
> - return 256UL * 1024 * 1024;
> + return 256UL * 1024 * 1024;
> }
> #endif
>
^ permalink raw reply
* [PATCH kernel 0/2] KVM: PPC: Allow backing bigger guest IOMMU pages with smaller physical
From: Alexey Kardashevskiy @ 2018-05-02 4:07 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Alexey Kardashevskiy, David Gibson, kvm-ppc, Paul Mackerras
This is to allow the guest to use big host IOMMU pages even when
the exact page size (i.e. 16MB) is not supported by the host (i.e.P9).
Please comment. Thanks.
Alexey Kardashevskiy (2):
KVM: PPC: Use correct page shift in H_STUFF_TCE
KVM: PPC: Allow backing bigger guest IOMMU pages with smaller physical
pages
arch/powerpc/kvm/book3s_64_vio.c | 68 +++++++++++++++++++++++++++++--------
arch/powerpc/kvm/book3s_64_vio_hv.c | 54 +++++++++++++++++++++++++----
2 files changed, 100 insertions(+), 22 deletions(-)
--
2.11.0
^ permalink raw reply
* [PATCH kernel 2/2] KVM: PPC: Allow backing bigger guest IOMMU pages with smaller physical pages
From: Alexey Kardashevskiy @ 2018-05-02 4:07 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Alexey Kardashevskiy, David Gibson, kvm-ppc, Paul Mackerras
In-Reply-To: <20180502040723.20545-1-aik@ozlabs.ru>
At the moment we only support in the host the IOMMU page sizes which
the guest is aware of, which is 4KB/64KB/16MB. However P9 does not support
16MB IOMMU pages, 2MB and 1GB pages are supported instead. We can still
emulate bigger guest pages (for example 16MB) with smaller host pages
(4KB/64KB/2MB).
This allows the physical IOMMU pages to use a page size smaller or equal
than the guest visible IOMMU page size.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
arch/powerpc/kvm/book3s_64_vio.c | 66 +++++++++++++++++++++++++++++--------
arch/powerpc/kvm/book3s_64_vio_hv.c | 52 +++++++++++++++++++++++++----
2 files changed, 98 insertions(+), 20 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_64_vio.c b/arch/powerpc/kvm/book3s_64_vio.c
index 041e54d..e10d6a3 100644
--- a/arch/powerpc/kvm/book3s_64_vio.c
+++ b/arch/powerpc/kvm/book3s_64_vio.c
@@ -176,14 +176,12 @@ extern long kvm_spapr_tce_attach_iommu_group(struct kvm *kvm, int tablefd,
if (!tbltmp)
continue;
- /*
- * Make sure hardware table parameters are exactly the same;
- * this is used in the TCE handlers where boundary checks
- * use only the first attached table.
- */
- if ((tbltmp->it_page_shift == stt->page_shift) &&
- (tbltmp->it_offset == stt->offset) &&
- (tbltmp->it_size == stt->size)) {
+ /* Make sure hardware table parameters are compatible */
+ if ((tbltmp->it_page_shift <= stt->page_shift) &&
+ (tbltmp->it_offset << tbltmp->it_page_shift ==
+ stt->offset << stt->page_shift) &&
+ (tbltmp->it_size << tbltmp->it_page_shift ==
+ stt->size << stt->page_shift)) {
/*
* Reference the table to avoid races with
* add/remove DMA windows.
@@ -396,7 +394,7 @@ static long kvmppc_tce_iommu_mapped_dec(struct kvm *kvm,
return H_SUCCESS;
}
-static long kvmppc_tce_iommu_unmap(struct kvm *kvm,
+static long kvmppc_tce_iommu_do_unmap(struct kvm *kvm,
struct iommu_table *tbl, unsigned long entry)
{
enum dma_data_direction dir = DMA_NONE;
@@ -416,7 +414,25 @@ static long kvmppc_tce_iommu_unmap(struct kvm *kvm,
return ret;
}
-long kvmppc_tce_iommu_map(struct kvm *kvm, struct iommu_table *tbl,
+static long kvmppc_tce_iommu_unmap(struct kvm *kvm,
+ struct kvmppc_spapr_tce_table *stt, struct iommu_table *tbl,
+ unsigned long entry)
+{
+ unsigned long ret = H_SUCCESS;
+ unsigned long subpages = 1ULL << (stt->page_shift - tbl->it_page_shift);
+ unsigned long io_entry = entry * subpages;
+ unsigned long subpg;
+
+ for (subpg = 0; subpg < subpages; ++subpg) {
+ ret = kvmppc_tce_iommu_do_unmap(kvm, tbl, io_entry + subpg);
+ if (ret != H_SUCCESS)
+ break;
+ }
+
+ return ret;
+}
+
+long kvmppc_tce_iommu_do_map(struct kvm *kvm, struct iommu_table *tbl,
unsigned long entry, unsigned long ua,
enum dma_data_direction dir)
{
@@ -453,6 +469,28 @@ long kvmppc_tce_iommu_map(struct kvm *kvm, struct iommu_table *tbl,
return 0;
}
+static long kvmppc_tce_iommu_map(struct kvm *kvm,
+ struct kvmppc_spapr_tce_table *stt, struct iommu_table *tbl,
+ unsigned long entry, unsigned long ua,
+ enum dma_data_direction dir)
+{
+ unsigned long ret = H_SUCCESS;
+ unsigned long subpages = 1ULL << (stt->page_shift - tbl->it_page_shift);
+ unsigned long io_entry = entry * subpages;
+ unsigned long subpg, pgoff;
+
+ for (subpg = 0, pgoff = 0; subpg < subpages;
+ ++subpg, pgoff += IOMMU_PAGE_SIZE(tbl)) {
+
+ ret = kvmppc_tce_iommu_do_map(kvm, tbl,
+ io_entry + subpg, ua + pgoff, dir);
+ if (ret != H_SUCCESS)
+ break;
+ }
+
+ return ret;
+}
+
long kvmppc_h_put_tce(struct kvm_vcpu *vcpu, unsigned long liobn,
unsigned long ioba, unsigned long tce)
{
@@ -491,10 +529,10 @@ long kvmppc_h_put_tce(struct kvm_vcpu *vcpu, unsigned long liobn,
list_for_each_entry_lockless(stit, &stt->iommu_tables, next) {
if (dir == DMA_NONE)
- ret = kvmppc_tce_iommu_unmap(vcpu->kvm,
+ ret = kvmppc_tce_iommu_unmap(vcpu->kvm, stt,
stit->tbl, entry);
else
- ret = kvmppc_tce_iommu_map(vcpu->kvm, stit->tbl,
+ ret = kvmppc_tce_iommu_map(vcpu->kvm, stt, stit->tbl,
entry, ua, dir);
if (ret == H_SUCCESS)
@@ -570,7 +608,7 @@ long kvmppc_h_put_tce_indirect(struct kvm_vcpu *vcpu,
return H_PARAMETER;
list_for_each_entry_lockless(stit, &stt->iommu_tables, next) {
- ret = kvmppc_tce_iommu_map(vcpu->kvm,
+ ret = kvmppc_tce_iommu_map(vcpu->kvm, stt,
stit->tbl, entry + i, ua,
iommu_tce_direction(tce));
@@ -618,7 +656,7 @@ long kvmppc_h_stuff_tce(struct kvm_vcpu *vcpu,
unsigned long entry = ioba >> stt->page_shift;
for (i = 0; i < npages; ++i) {
- ret = kvmppc_tce_iommu_unmap(vcpu->kvm,
+ ret = kvmppc_tce_iommu_unmap(vcpu->kvm, stt,
stit->tbl, entry + i);
if (ret == H_SUCCESS)
diff --git a/arch/powerpc/kvm/book3s_64_vio_hv.c b/arch/powerpc/kvm/book3s_64_vio_hv.c
index e220fab..258e786 100644
--- a/arch/powerpc/kvm/book3s_64_vio_hv.c
+++ b/arch/powerpc/kvm/book3s_64_vio_hv.c
@@ -221,7 +221,7 @@ static long kvmppc_rm_tce_iommu_mapped_dec(struct kvm *kvm,
return H_SUCCESS;
}
-static long kvmppc_rm_tce_iommu_unmap(struct kvm *kvm,
+static long kvmppc_rm_tce_iommu_do_unmap(struct kvm *kvm,
struct iommu_table *tbl, unsigned long entry)
{
enum dma_data_direction dir = DMA_NONE;
@@ -245,7 +245,25 @@ static long kvmppc_rm_tce_iommu_unmap(struct kvm *kvm,
return ret;
}
-static long kvmppc_rm_tce_iommu_map(struct kvm *kvm, struct iommu_table *tbl,
+static long kvmppc_rm_tce_iommu_unmap(struct kvm *kvm,
+ struct kvmppc_spapr_tce_table *stt, struct iommu_table *tbl,
+ unsigned long entry)
+{
+ unsigned long ret = H_SUCCESS;
+ unsigned long subpages = 1ULL << (stt->page_shift - tbl->it_page_shift);
+ unsigned long io_entry = entry * subpages;
+ unsigned long subpg;
+
+ for (subpg = 0; subpg < subpages; ++subpg) {
+ ret = kvmppc_rm_tce_iommu_do_unmap(kvm, tbl, io_entry + subpg);
+ if (ret != H_SUCCESS)
+ break;
+ }
+
+ return ret;
+}
+
+static long kvmppc_rm_tce_iommu_do_map(struct kvm *kvm, struct iommu_table *tbl,
unsigned long entry, unsigned long ua,
enum dma_data_direction dir)
{
@@ -290,6 +308,28 @@ static long kvmppc_rm_tce_iommu_map(struct kvm *kvm, struct iommu_table *tbl,
return 0;
}
+static long kvmppc_rm_tce_iommu_map(struct kvm *kvm,
+ struct kvmppc_spapr_tce_table *stt, struct iommu_table *tbl,
+ unsigned long entry, unsigned long ua,
+ enum dma_data_direction dir)
+{
+ unsigned long ret = H_SUCCESS;
+ unsigned long subpages = 1ULL << (stt->page_shift - tbl->it_page_shift);
+ unsigned long io_entry = entry * subpages;
+ unsigned long subpg, pgoff;
+
+ for (subpg = 0, pgoff = 0; subpg < subpages;
+ ++subpg, pgoff += IOMMU_PAGE_SIZE(tbl)) {
+
+ ret = kvmppc_rm_tce_iommu_do_map(kvm, tbl,
+ io_entry + subpg, ua + pgoff, dir);
+ if (ret != H_SUCCESS)
+ break;
+ }
+
+ return ret;
+}
+
long kvmppc_rm_h_put_tce(struct kvm_vcpu *vcpu, unsigned long liobn,
unsigned long ioba, unsigned long tce)
{
@@ -327,10 +367,10 @@ long kvmppc_rm_h_put_tce(struct kvm_vcpu *vcpu, unsigned long liobn,
list_for_each_entry_lockless(stit, &stt->iommu_tables, next) {
if (dir == DMA_NONE)
- ret = kvmppc_rm_tce_iommu_unmap(vcpu->kvm,
+ ret = kvmppc_rm_tce_iommu_unmap(vcpu->kvm, stt,
stit->tbl, entry);
else
- ret = kvmppc_rm_tce_iommu_map(vcpu->kvm,
+ ret = kvmppc_rm_tce_iommu_map(vcpu->kvm, stt,
stit->tbl, entry, ua, dir);
if (ret == H_SUCCESS)
@@ -477,7 +517,7 @@ long kvmppc_rm_h_put_tce_indirect(struct kvm_vcpu *vcpu,
return H_PARAMETER;
list_for_each_entry_lockless(stit, &stt->iommu_tables, next) {
- ret = kvmppc_rm_tce_iommu_map(vcpu->kvm,
+ ret = kvmppc_rm_tce_iommu_map(vcpu->kvm, stt,
stit->tbl, entry + i, ua,
iommu_tce_direction(tce));
@@ -529,7 +569,7 @@ long kvmppc_rm_h_stuff_tce(struct kvm_vcpu *vcpu,
unsigned long entry = ioba >> stt->page_shift;
for (i = 0; i < npages; ++i) {
- ret = kvmppc_rm_tce_iommu_unmap(vcpu->kvm,
+ ret = kvmppc_rm_tce_iommu_unmap(vcpu->kvm, stt,
stit->tbl, entry + i);
if (ret == H_SUCCESS)
--
2.11.0
^ permalink raw reply related
* [PATCH kernel 1/2] KVM: PPC: Use correct page shift in H_STUFF_TCE
From: Alexey Kardashevskiy @ 2018-05-02 4:07 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Alexey Kardashevskiy, David Gibson, kvm-ppc, Paul Mackerras
In-Reply-To: <20180502040723.20545-1-aik@ozlabs.ru>
The other TCE handlers use page shift from the guest visible TCE table
(described by kvmppc_spapr_tce_iommu_table) so let's make H_STUFF_TCE
handlers do the same thing.
This should cause no behavioral change now but soon we will allow
the iommu_table::it_page_shift being different from from the emulated
table page size so this will play a role.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
arch/powerpc/kvm/book3s_64_vio.c | 2 +-
arch/powerpc/kvm/book3s_64_vio_hv.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_64_vio.c b/arch/powerpc/kvm/book3s_64_vio.c
index 4dffa61..041e54d 100644
--- a/arch/powerpc/kvm/book3s_64_vio.c
+++ b/arch/powerpc/kvm/book3s_64_vio.c
@@ -615,7 +615,7 @@ long kvmppc_h_stuff_tce(struct kvm_vcpu *vcpu,
return H_PARAMETER;
list_for_each_entry_lockless(stit, &stt->iommu_tables, next) {
- unsigned long entry = ioba >> stit->tbl->it_page_shift;
+ unsigned long entry = ioba >> stt->page_shift;
for (i = 0; i < npages; ++i) {
ret = kvmppc_tce_iommu_unmap(vcpu->kvm,
diff --git a/arch/powerpc/kvm/book3s_64_vio_hv.c b/arch/powerpc/kvm/book3s_64_vio_hv.c
index 6651f73..e220fab 100644
--- a/arch/powerpc/kvm/book3s_64_vio_hv.c
+++ b/arch/powerpc/kvm/book3s_64_vio_hv.c
@@ -526,7 +526,7 @@ long kvmppc_rm_h_stuff_tce(struct kvm_vcpu *vcpu,
return H_PARAMETER;
list_for_each_entry_lockless(stit, &stt->iommu_tables, next) {
- unsigned long entry = ioba >> stit->tbl->it_page_shift;
+ unsigned long entry = ioba >> stt->page_shift;
for (i = 0; i < npages; ++i) {
ret = kvmppc_rm_tce_iommu_unmap(vcpu->kvm,
--
2.11.0
^ permalink raw reply related
* Re: [PATCHv2 0/3] post the event cpux add/remove besides online/offline during hotplug
From: Pingfan Liu @ 2018-05-02 2:43 UTC (permalink / raw)
To: linuxppc-dev
Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Hari Bathini
In-Reply-To: <1523860508-19364-1-git-send-email-kernelfans@gmail.com>
Maintainers, ping? Any suggestion? I encounter such issue on redhat
RHEL and FEDORA
On Mon, Apr 16, 2018 at 2:35 PM, Pingfan Liu <kernelfans@gmail.com> wrote:
> v1->v2:
> -1.improve the commit log and explain the reproducing of bug in [3/3]
> -2.re-fragment the series, and [3/3] is the motivation, while [1~2/3] are preparation.
>
> Pingfan Liu (3):
> powerpc/cpuidle: dynamically register/unregister cpuidle_device during
> hotplug
> powerpc/cpu: dynmamically to create/destroy the file physical_id
> during hotplug
> powerpc/cpu: post the event cpux add/remove instead of online/offline
> during hotplug
>
> arch/powerpc/include/asm/smp.h | 1 +
> arch/powerpc/kernel/sysfs.c | 26 ++++++++++++++------------
> arch/powerpc/platforms/pseries/hotplug-cpu.c | 3 +++
> drivers/cpuidle/cpuidle-powernv.c | 2 ++
> drivers/cpuidle/cpuidle-pseries.c | 2 ++
> 5 files changed, 22 insertions(+), 12 deletions(-)
>
> --
> 2.7.4
>
^ permalink raw reply
* Re: [RESEND 2/3] powerpc/memcpy: Add memcpy_mcsafe for pmem
From: Dan Williams @ 2018-05-01 20:57 UTC (permalink / raw)
To: Nicholas Piggin
Cc: Balbir Singh, Michael Ellerman, linuxppc-dev, linux-nvdimm,
Christoph Hellwig, Matthew Wilcox, Luck, Tony
In-Reply-To: <CAPcyv4hQDetYZaQXHoD-L+KWrMJyypvHw=xzm7tb61qt9w=a3g@mail.gmail.com>
On Thu, Apr 5, 2018 at 8:00 AM, Dan Williams <dan.j.williams@intel.com> wrote:
> On Wed, Apr 4, 2018 at 11:45 PM, Nicholas Piggin <npiggin@gmail.com> wrote:
[,,]
>> What's the problem with just counting bytes copied like usercopy --
>> why is that harder than cacheline accuracy?
>>
>>> I'd rather implement the existing interface and port/support the new interface
>>> as it becomes available
>>
>> Fair enough.
>
> I have patches already in progress to change the interface. My
> preference is to hold off on adding a new implementation that will
> need to be immediately reworked. When I say "immediate" I mean that
> should be able to post what I have for review within the next few
> days.
>
> Whether this is all too late for 4.17 is another question...
Here is the x86 version of a 'bytes remaining' memcpy_mcsafe() implemenation:
https://lists.01.org/pipermail/linux-nvdimm/2018-May/015548.html
^ permalink raw reply
* Re: [RFC] virtio: Use DMA MAP API for devices without an IOMMU
From: Ram Pai @ 2018-05-01 16:34 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Anshuman Khandual, robh, aik, jasowang, linux-kernel,
virtualization, Christoph Hellwig, joe, linuxppc-dev, elfring,
david
In-Reply-To: <20180418191722-mutt-send-email-mst@kernel.org>
On Wed, Apr 18, 2018 at 07:20:10PM +0300, Michael S. Tsirkin wrote:
> On Wed, Apr 18, 2018 at 08:47:10AM +0530, Anshuman Khandual wrote:
> > On 04/15/2018 05:41 PM, Christoph Hellwig wrote:
> > > On Fri, Apr 06, 2018 at 06:37:18PM +1000, Benjamin Herrenschmidt wrote:
> > >>>> implemented as DMA API which the virtio core understands. There is no
> > >>>> need for an IOMMU to be involved for the device representation in this
> > >>>> case IMHO.
> > >>>
> > >>> This whole virtio translation issue is a mess. I think we need to
> > >>> switch it to the dma API, and then quirk the legacy case to always
> > >>> use the direct mapping inside the dma API.
> > >>
> > >> Fine with using a dma API always on the Linux side, but we do want to
> > >> special case virtio still at the arch and qemu side to have a "direct
> > >> mapping" mode. Not sure how (special flags on PCI devices) to avoid
> > >> actually going through an emulated IOMMU on the qemu side, because that
> > >> slows things down, esp. with vhost.
> > >>
> > >> IE, we can't I think just treat it the same as a physical device.
> > >
> > > We should have treated it like a physical device from the start, but
> > > that device has unfortunately sailed.
> > >
> > > But yes, we'll need a per-device quirk that says 'don't attach an
> > > iommu'.
> >
> > How about doing it per platform basis as suggested in this RFC through
> > an arch specific callback. Because all the virtio devices in the given
> > platform would require and exercise this option (to avail bounce buffer
> > mechanism for secure guests as an example). So the flag basically is a
> > platform specific one not a device specific one.
>
> That's not the case. A single platform can have a mix of virtio and
> non-virtio devices. Same applies even within virtio, e.g. the balloon
> device always bypasses an iommu. Further, QEMU supports out of process
> devices some of which might bypass the IOMMU.
Given that each virtio device has to behave differently depending on
(a) what it does? (balloon, block, net etc )
(b) what platform it is on? (pseries, x86, ....)
(c) what environment it is on? (secure, insecure...)
I think, we should let the virtio device decide what it wants, instead
of forcing it to NOT use dma_ops when VIRTIO_F_IOMMU_PLATFORM is NOT
enabled.
Currently, virtio generic code, has an assumption that a device must NOT
use dma operations if the hypervisor has NOT enabled VIRTIO_F_IOMMU_PLATFORM.
This assumption is baked into vring_use_dma_api(); though there is a
special exception for xen_domain().
This assumption is restricting us from using the dma_ops abstraction for
virtio devices on secure VM. BTW: VIRTIO_F_IOMMU_PLATFORM may or may not
be set on this platform.
On our secure VM, virtio devices; by default, do not share pages with
hypervisor. In other words, hypervisor cannot access the secure VM
pages. The secure VM with the help of the hardware enables some pages
to be shared with the hypervisor. Secure VM then uses these pages to
bounce virtio data with the hypervisor.
One elegant way to impliment this functionality is to abstract it
under our special dma_ops and wire it to the virtio devices.
However the restriction imposed by the generic virtio code, contrains us
from doing so.
If we can enrich vring_use_dma_api() to take multiple factors into
consideration and not just VIRTIO_F_IOMMU_PLATFORM; perferrably by
consulting a arch-dependent function, we could seemlessly integrate
into the existing virtio infrastructure.
RP
>
> --
> MST
--
Ram Pai
^ permalink raw reply
* Re: [PATCH v10 08/25] mm: VMA sequence count
From: Minchan Kim @ 2018-05-01 13:16 UTC (permalink / raw)
To: Laurent Dufour
Cc: akpm, mhocko, peterz, kirill, ak, dave, jack, Matthew Wilcox,
benh, mpe, paulus, Thomas Gleixner, Ingo Molnar, hpa, Will Deacon,
Sergey Senozhatsky, Andrea Arcangeli, Alexei Starovoitov,
kemi.wang, sergey.senozhatsky.work, Daniel Jordan, David Rientjes,
Jerome Glisse, Ganesh Mahendran, linux-kernel, linux-mm, haren,
khandual, npiggin, bsingharora, paulmck, Tim Chen, linuxppc-dev,
x86
In-Reply-To: <06b996b0-b831-3d39-8a99-792abfb6a4d1@linux.vnet.ibm.com>
On Mon, Apr 30, 2018 at 05:14:27PM +0200, Laurent Dufour wrote:
>
>
> On 23/04/2018 08:42, Minchan Kim wrote:
> > On Tue, Apr 17, 2018 at 04:33:14PM +0200, Laurent Dufour wrote:
> >> From: Peter Zijlstra <peterz@infradead.org>
> >>
> >> Wrap the VMA modifications (vma_adjust/unmap_page_range) with sequence
> >> counts such that we can easily test if a VMA is changed.
> >
> > So, seqcount is to protect modifying all attributes of vma?
>
> The seqcount is used to protect fields that will be used during the speculative
> page fault like boundaries, protections.
a VMA is changed, it was rather vague to me at this point.
If you could specify detail fields or some example what seqcount aim for,
it would help to review.
>
> >>
> >> The unmap_page_range() one allows us to make assumptions about
> >> page-tables; when we find the seqcount hasn't changed we can assume
> >> page-tables are still valid.
> >
> > Hmm, seqcount covers page-table, too.
> > Please describe what the seqcount want to protect.
>
> The calls to vm_write_begin/end() in unmap_page_range() are used to detect when
> a VMA is being unmap and thus that new page fault should not be satisfied for
> this VMA. This is protecting the VMA unmapping operation, not the page tables
> themselves.
Thanks for the detail. yes, please include this phrase instead of "page-table
are still valid". It makes me confused.
>
> >>
> >> The flip side is that we cannot distinguish between a vma_adjust() and
> >> the unmap_page_range() -- where with the former we could have
> >> re-checked the vma bounds against the address.
> >>
> >> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> >>
> >> [Port to 4.12 kernel]
> >> [Build depends on CONFIG_SPECULATIVE_PAGE_FAULT]
> >> [Introduce vm_write_* inline function depending on
> >> CONFIG_SPECULATIVE_PAGE_FAULT]
> >> [Fix lock dependency between mapping->i_mmap_rwsem and vma->vm_sequence by
> >> using vm_raw_write* functions]
> >> [Fix a lock dependency warning in mmap_region() when entering the error
> >> path]
> >> [move sequence initialisation INIT_VMA()]
> >> Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
> >> ---
> >> include/linux/mm.h | 44 ++++++++++++++++++++++++++++++++++++++++++++
> >> include/linux/mm_types.h | 3 +++
> >> mm/memory.c | 2 ++
> >> mm/mmap.c | 31 +++++++++++++++++++++++++++++++
> >> 4 files changed, 80 insertions(+)
> >>
> >> diff --git a/include/linux/mm.h b/include/linux/mm.h
> >> index efc1248b82bd..988daf7030c9 100644
> >> --- a/include/linux/mm.h
> >> +++ b/include/linux/mm.h
> >> @@ -1264,6 +1264,9 @@ struct zap_details {
> >> static inline void INIT_VMA(struct vm_area_struct *vma)
> >> {
> >> INIT_LIST_HEAD(&vma->anon_vma_chain);
> >> +#ifdef CONFIG_SPECULATIVE_PAGE_FAULT
> >> + seqcount_init(&vma->vm_sequence);
> >> +#endif
> >> }
> >>
> >> struct page *_vm_normal_page(struct vm_area_struct *vma, unsigned long addr,
> >> @@ -1386,6 +1389,47 @@ static inline void unmap_shared_mapping_range(struct address_space *mapping,
> >> unmap_mapping_range(mapping, holebegin, holelen, 0);
> >> }
> >>
> >> +#ifdef CONFIG_SPECULATIVE_PAGE_FAULT
> >> +static inline void vm_write_begin(struct vm_area_struct *vma)
> >> +{
> >> + write_seqcount_begin(&vma->vm_sequence);
> >> +}
> >> +static inline void vm_write_begin_nested(struct vm_area_struct *vma,
> >> + int subclass)
> >> +{
> >> + write_seqcount_begin_nested(&vma->vm_sequence, subclass);
> >> +}
> >> +static inline void vm_write_end(struct vm_area_struct *vma)
> >> +{
> >> + write_seqcount_end(&vma->vm_sequence);
> >> +}
> >> +static inline void vm_raw_write_begin(struct vm_area_struct *vma)
> >> +{
> >> + raw_write_seqcount_begin(&vma->vm_sequence);
> >> +}
> >> +static inline void vm_raw_write_end(struct vm_area_struct *vma)
> >> +{
> >> + raw_write_seqcount_end(&vma->vm_sequence);
> >> +}
> >> +#else
> >> +static inline void vm_write_begin(struct vm_area_struct *vma)
> >> +{
> >> +}
> >> +static inline void vm_write_begin_nested(struct vm_area_struct *vma,
> >> + int subclass)
> >> +{
> >> +}
> >> +static inline void vm_write_end(struct vm_area_struct *vma)
> >> +{
> >> +}
> >> +static inline void vm_raw_write_begin(struct vm_area_struct *vma)
> >> +{
> >> +}
> >> +static inline void vm_raw_write_end(struct vm_area_struct *vma)
> >> +{
> >> +}
> >> +#endif /* CONFIG_SPECULATIVE_PAGE_FAULT */
> >> +
> >> extern int access_process_vm(struct task_struct *tsk, unsigned long addr,
> >> void *buf, int len, unsigned int gup_flags);
> >> extern int access_remote_vm(struct mm_struct *mm, unsigned long addr,
> >> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> >> index 21612347d311..db5e9d630e7a 100644
> >> --- a/include/linux/mm_types.h
> >> +++ b/include/linux/mm_types.h
> >> @@ -335,6 +335,9 @@ struct vm_area_struct {
> >> struct mempolicy *vm_policy; /* NUMA policy for the VMA */
> >> #endif
> >> struct vm_userfaultfd_ctx vm_userfaultfd_ctx;
> >> +#ifdef CONFIG_SPECULATIVE_PAGE_FAULT
> >> + seqcount_t vm_sequence;
> >> +#endif
> >> } __randomize_layout;
> >>
> >> struct core_thread {
> >> diff --git a/mm/memory.c b/mm/memory.c
> >> index f86efcb8e268..f7fed053df80 100644
> >> --- a/mm/memory.c
> >> +++ b/mm/memory.c
> >> @@ -1503,6 +1503,7 @@ void unmap_page_range(struct mmu_gather *tlb,
> >> unsigned long next;
> >>
> >> BUG_ON(addr >= end);
> >
> > The comment about saying it aims for page-table stability will help.
>
> A comment may be added mentioning that we use the seqcount to indicate that the
> VMA is modified, being unmapped. But there is not a real page table protection,
> and I think this may be confusing to talk about page table stability here.
Okay, so here you mean seqcount is not protecting VMA's fields but vma unmap
operation like you mentioned above. I was confused like below description.
"The unmap_page_range() one allows us to make assumptions about
page-tables; when we find the seqcount hasn't changed we can assume
page-tables are still valid"
Instead of using page-tables's validness in descriptoin, it would be better
to use scenario you mentioned about VMA unmap operation and page fault race.
Thanks.
^ permalink raw reply
* Re: [PATCH] powerpc/watchdog: provide more data in watchdog messages
From: Balbir Singh @ 2018-05-01 13:07 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev
In-Reply-To: <20180501022216.1420-1-npiggin@gmail.com>
On Tue, 2018-05-01 at 12:22 +1000, Nicholas Piggin wrote:
> Provide timebase and timebase of last heartbeat in watchdog lockup
> messages. Also provide a stack trace of when a CPU becomes un-stuck,
> which can be useful -- it could be where irqs are re-enabled, so it
> may be the end of the critical section which is responsible for the
> latency.
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>
> A lockup + unstuck event now looks like this (with irqtrace enabled):
>
> watchdog: CPU 1 self-detected hard LOCKUP @ udelay+0x40/0x60
> watchdog: CPU 1 TB:82611697355, last heartbeat TB:75431975757
Can we divide TB with tb_ticks_per_sec, TB itself is not very useful, the
delta maybe, but it needs more work on behalf of the person looking
at the output.
> Modules linked in:
> irq event stamp: 2250184
> hardirqs last enabled at (2250183): [<c000000000bb9158>] _raw_spin_unlock_irqrestore+0x58/0xd0
> hardirqs last disabled at (2250184): [<c00000000000e76c>] kernel_init+0x7c/0x1b0
> softirqs last enabled at (2243178): [<c000000000bba5d8>] __do_softirq+0x4d8/0x6f8
> softirqs last disabled at (2243159): [<c0000000000f4138>] irq_exit+0x108/0x1a0
> CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.17.0-rc3-00003-g7e2de68fdf39 #319
> NIP: c000000000026d40 LR: c00000000000e77c CTR: 0000000000000000
> REGS: c000000fffcebd80 TRAP: 0900 Not tainted (4.17.0-rc3-00003-g7e2de68fdf39)
> MSR: 9000000002009033 <SF,HV,VEC,EE,ME,IR,DR,RI,LE> CR: 44000824 XER: 20040000
> CFAR: c000000000026d4c SOFTE: 1
> GPR00: c00000000000e77c c000000006a07dc0 c000000001306000 00000002625a0000
> GPR04: c00000000000e76c 0000000000000038 8f5c28f5c28f5c29 c000000ff0e18cb0
> GPR08: c0000000011a6000 00000001802d8dcf 00000011bbdcdf4c 7bc4830ddfa301bf
> GPR12: 0000000000000000 c000000fffffe300 c00000000000e6f8 0000000000000000
> GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> GPR20: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> GPR24: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> GPR28: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> NIP [c000000000026d40] udelay+0x40/0x60
> LR [c00000000000e77c] kernel_init+0x8c/0x1b0
> Call Trace:
> [c000000006a07dc0] [c00000000000e76c] kernel_init+0x7c/0x1b0 (unreliable)
> [c000000006a07e30] [c00000000000b9b0] ret_from_kernel_thread+0x5c/0xac
> Instruction dump:
> 3d22ffe9 e929d400 7c6349d2 7c210b78 7d4c42a6 7d2c42a6 7d2a4850 7fa34840
> 409d0020 60000000 60000000 60000000 <7d2c42a6> 7d2a4850 7fa34840 419dfff4
> time 86406258554
> watchdog: CPU 1 became unstuck TB:86406275034
Same as above for TB
> irq event stamp: 2250201
> hardirqs last enabled at (2250200): [<c000000000bb9218>] _raw_spin_unlock_irq+0x48/0x80
> hardirqs last disabled at (2250201): [<c000000000189ff8>] vprintk_emit+0x98/0x5c0
> softirqs last enabled at (2243178): [<c000000000bba5d8>] __do_softirq+0x4d8/0x6f8
> softirqs last disabled at (2250191): [<c0000000000f4138>] irq_exit+0x108/0x1a0
> CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.17.0-rc3-00003-g7e2de68fdf39 #319
> NIP: c00000000000abdc LR: c0000000000186b8 CTR: 000000003003e65c
> REGS: c000000006a07b10 TRAP: 0901 Not tainted (4.17.0-rc3-00003-g7e2de68fdf39)
> MSR: 9000000002009033 <SF,HV,VEC,EE,ME,IR,DR,RI,LE> CR: 48000824 XER: 00000000
> CFAR: c000000000026d4c SOFTE: 0
> GPR00: c000000000018660 c000000006a07d90 c000000001306000 0000000000000900
> GPR04: 0000000000000001 0000000000000038 8f5c28f5c28f5c29 c000000ff0e17970
> GPR08: c000000001348d98 00000002002d8dee 00000011bbdcdf4c 7bc4830ddfa301bf
> GPR12: 0000000000000000 c000000fffffe300
> NIP [c00000000000abdc] replay_interrupt_return+0x0/0x4
> LR [c0000000000186b8] arch_local_irq_restore+0xd8/0xf0
> Call Trace:
> [c000000006a07d90] [c000000000018660] arch_local_irq_restore+0x80/0xf0 (unreliable)
> [c000000006a07dc0] [c00000000000e7ac] kernel_init+0xbc/0x1b0
> [c000000006a07e30] [c00000000000b9b0] ret_from_kernel_thread+0x5c/0xac
> Instruction dump:
> 7d200026 618c8000 2c030900 4182e518 2c030500 4182f140 2c030f00 4182f278
> 2c030a00 4182ff9c 2c030e60 4182eea8 <4e800020> 7c781b78 480003a5 480003bd
>
>
> arch/powerpc/kernel/watchdog.c | 26 +++++++++++++++++++++-----
> 1 file changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/kernel/watchdog.c b/arch/powerpc/kernel/watchdog.c
> index 6256dc3b0087..8a0f7d97f8d9 100644
> --- a/arch/powerpc/kernel/watchdog.c
> +++ b/arch/powerpc/kernel/watchdog.c
> @@ -111,7 +111,11 @@ static inline void wd_smp_unlock(unsigned long *flags)
>
> static void wd_lockup_ipi(struct pt_regs *regs)
> {
> - pr_emerg("CPU %d Hard LOCKUP\n", raw_smp_processor_id());
> + int cpu = raw_smp_processor_id();
> +
> + pr_emerg("CPU %d Hard LOCKUP\n", cpu);
> + pr_emerg("CPU %d TB:%lld, last heartbeat TB:%lld\n",
> + cpu, get_tb(), per_cpu(wd_timer_tb, cpu));
> print_modules();
> print_irqtrace_events(current);
> if (regs)
> @@ -154,6 +158,8 @@ static void watchdog_smp_panic(int cpu, u64 tb)
>
> pr_emerg("CPU %d detected hard LOCKUP on other CPUs %*pbl\n",
> cpu, cpumask_pr_args(&wd_smp_cpus_pending));
> + pr_emerg("CPU %d TB:%lld, last SMP heartbeat TB:%lld\n",
> + cpu, tb, wd_smp_last_reset_tb);
>
> if (!sysctl_hardlockup_all_cpu_backtrace) {
> /*
> @@ -194,10 +200,19 @@ static void wd_smp_clear_cpu_pending(int cpu, u64 tb)
> {
> if (!cpumask_test_cpu(cpu, &wd_smp_cpus_pending)) {
> if (unlikely(cpumask_test_cpu(cpu, &wd_smp_cpus_stuck))) {
> + struct pt_regs *regs = get_irq_regs();
> unsigned long flags;
>
> - pr_emerg("CPU %d became unstuck\n", cpu);
> wd_smp_lock(&flags);
> +
> + pr_emerg("CPU %d became unstuck TB:%lld\n",
> + cpu, tb);
> + print_irqtrace_events(current);
> + if (regs)
> + show_regs(regs);
> + else
> + dump_stack();
> +
> cpumask_clear_cpu(cpu, &wd_smp_cpus_stuck);
> wd_smp_unlock(&flags);
> }
> @@ -245,8 +260,6 @@ void soft_nmi_interrupt(struct pt_regs *regs)
>
> tb = get_tb();
> if (tb - per_cpu(wd_timer_tb, cpu) >= wd_panic_timeout_tb) {
> - per_cpu(wd_timer_tb, cpu) = tb;
> -
Is this related to the print improvements? It looks like you don't want
to reset the tb, but I would split it out
> wd_smp_lock(&flags);
> if (cpumask_test_cpu(cpu, &wd_smp_cpus_stuck)) {
> wd_smp_unlock(&flags);
> @@ -254,7 +267,10 @@ void soft_nmi_interrupt(struct pt_regs *regs)
> }
> set_cpu_stuck(cpu, tb);
>
> - pr_emerg("CPU %d self-detected hard LOCKUP @ %pS\n", cpu, (void *)regs->nip);
> + pr_emerg("CPU %d self-detected hard LOCKUP @ %pS\n",
> + cpu, (void *)regs->nip);
> + pr_emerg("CPU %d TB:%lld, last heartbeat TB:%lld\n",
> + cpu, get_tb(), per_cpu(wd_timer_tb, cpu));
> print_modules();
> print_irqtrace_events(current);
> show_regs(regs);
Balbir Singh.
^ permalink raw reply
* Re: [PATCH v10 06/25] mm: make pte_unmap_same compatible with SPF
From: Minchan Kim @ 2018-05-01 13:04 UTC (permalink / raw)
To: Laurent Dufour
Cc: akpm, mhocko, peterz, kirill, ak, dave, jack, Matthew Wilcox,
benh, mpe, paulus, Thomas Gleixner, Ingo Molnar, hpa, Will Deacon,
Sergey Senozhatsky, Andrea Arcangeli, Alexei Starovoitov,
kemi.wang, sergey.senozhatsky.work, Daniel Jordan, David Rientjes,
Jerome Glisse, Ganesh Mahendran, linux-kernel, linux-mm, haren,
khandual, npiggin, bsingharora, paulmck, Tim Chen, linuxppc-dev,
x86
In-Reply-To: <dd5c4338-3cbb-c65a-f0c1-c71e2a0037ee@linux.vnet.ibm.com>
On Mon, Apr 30, 2018 at 04:07:30PM +0200, Laurent Dufour wrote:
> On 23/04/2018 08:31, Minchan Kim wrote:
> > On Tue, Apr 17, 2018 at 04:33:12PM +0200, Laurent Dufour wrote:
> >> pte_unmap_same() is making the assumption that the page table are still
> >> around because the mmap_sem is held.
> >> This is no more the case when running a speculative page fault and
> >> additional check must be made to ensure that the final page table are still
> >> there.
> >>
> >> This is now done by calling pte_spinlock() to check for the VMA's
> >> consistency while locking for the page tables.
> >>
> >> This is requiring passing a vm_fault structure to pte_unmap_same() which is
> >> containing all the needed parameters.
> >>
> >> As pte_spinlock() may fail in the case of a speculative page fault, if the
> >> VMA has been touched in our back, pte_unmap_same() should now return 3
> >> cases :
> >> 1. pte are the same (0)
> >> 2. pte are different (VM_FAULT_PTNOTSAME)
> >> 3. a VMA's changes has been detected (VM_FAULT_RETRY)
> >>
> >> The case 2 is handled by the introduction of a new VM_FAULT flag named
> >> VM_FAULT_PTNOTSAME which is then trapped in cow_user_page().
> >
> > I don't see such logic in this patch.
> > Maybe you introduces it later? If so, please comment on it.
> > Or just return 0 in case of 2 without introducing VM_FAULT_PTNOTSAME.
>
> Late in the series, pte_spinlock() will check for the VMA's changes and may
> return 1. This will be then required to handle the 3 cases presented above.
>
> I can move this handling later in the series, but I wondering if this will make
> it more easier to read.
Just nit:
During reviewing this patch, I was just curious you introduced new thing
here but I couldn't find any site where use it. It makes review hard. :(
That's why I said to you that please commet on it if you will use new thing
late in this series.
If you think as-is is better for review, it would be better to mention it
explicitly.
Thanks.
^ permalink raw reply
* Re: [PATCH v4 2/6] iommu: of: make of_pci_map_rid() available for other devices too
From: Rob Herring @ 2018-05-01 12:44 UTC (permalink / raw)
To: Nipun Gupta
Cc: robin.murphy, will.deacon, mark.rutland, catalin.marinas, gregkh,
hch, joro, m.szyprowski, shawnguo, frowand.list, bhelgaas, iommu,
linux-kernel, devicetree, linux-arm-kernel, linuxppc-dev,
linux-pci, bharat.bhushan, stuyoder, laurentiu.tudor, leoyang.li
In-Reply-To: <1525069641-8523-3-git-send-email-nipun.gupta@nxp.com>
On Mon, Apr 30, 2018 at 11:57:17AM +0530, Nipun Gupta wrote:
> iommu-map property is also used by devices with fsl-mc. This
> patch moves the of_pci_map_rid to generic location, so that it
> can be used by other busses too.
>
> 'of_pci_map_rid' is renamed here to 'of_map_rid' and there is no
> functional change done in the API.
>
> Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com>
> ---
> drivers/iommu/of_iommu.c | 5 +--
> drivers/of/base.c | 102 +++++++++++++++++++++++++++++++++++++++++++++++
> drivers/of/irq.c | 5 +--
> drivers/pci/of.c | 101 ----------------------------------------------
> include/linux/of.h | 11 +++++
> include/linux/of_pci.h | 10 -----
> 6 files changed, 117 insertions(+), 117 deletions(-)
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH 2/6] powerpc/64s: Add support for ori barrier_nospec patching
From: Michael Ellerman @ 2018-05-01 12:25 UTC (permalink / raw)
To: Michal Suchánek; +Cc: linuxppc-dev, npiggin, linux-kernel
In-Reply-To: <20180426181042.38971664@kitsune.suse.cz>
Michal Such=C3=A1nek <msuchanek@suse.de> writes:
> On Tue, 24 Apr 2018 14:15:55 +1000
> Michael Ellerman <mpe@ellerman.id.au> wrote:
>
>> From: Michal Suchanek <msuchanek@suse.de>
>>=20
>> Based on the RFI patching. This is required to be able to disable the
>> speculation barrier.
>
> why do you not patch the nospec barrier which is included as part of
> the RFI flush code?
We didn't want to put it in the asm like you had, because not all RFI
flush types need the spec barrier.
So it's more complicated than just patching in the spec barrier, we'd
need to only patch it in if the configured RFI flush also needed it.
> I think when debugging the code it would make more sense if RFI is
> patched by RFI patcher and nospec by nospec patcher.
True. I think what I'm saying is that the spec barrier in the RFI
flush is not a nospec barrier, it's part of that RFI flush type.
> A separate question is if the RFI flush would break without the nospec
> barrier.
The ORI flush requires it, but the others don't.
cheers
^ permalink raw reply
* Re: [PATCH 4/6] powerpc/64s: Enable barrier_nospec based on firmware settings
From: Michael Ellerman @ 2018-05-01 11:11 UTC (permalink / raw)
To: Michal Suchánek; +Cc: linuxppc-dev, linux-kernel, npiggin
In-Reply-To: <20180426180258.04e6bee6@kitsune.suse.cz>
Michal Such=C3=A1nek <msuchanek@suse.de> writes:
> Hello,
>
> On Tue, 24 Apr 2018 14:15:57 +1000
> Michael Ellerman <mpe@ellerman.id.au> wrote:
>
>> From: Michal Suchanek <msuchanek@suse.de>
>>=20
>> Check what firmware told us and enable/disable the barrier_nospec as
>> appropriate.
>>=20
>> We err on the side of enabling the barrier, as it's no-op on older
>> systems, see the comment for more detail.
>>=20
>> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
...
>
> I am missing the option for the barrier to be disabled by a kernel
> commandline argument here.
>
> It does make sense to add a kernel parameter that is checked on boot to
> be compatible with other platforms that implement one.
No other platforms have an option to disable variant 1 mitigations, so
there isn't an existing parameter we can use.
Which is not to say we can't add one, but I wasn't sure if it was really
worth it.
But I guess we should add one, I might do it as a follow-up patch.
cheers
^ permalink raw reply
* Re: [PATCH 08/15] powerpc/powernv: implement opal_put_chars_atomic
From: Nicholas Piggin @ 2018-05-01 10:37 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linuxppc-dev, Greg Kroah-Hartman, Jiri Slaby, linux-kernel
In-Reply-To: <1525168138.2325.100.camel@kernel.crashing.org>
On Tue, 01 May 2018 19:48:58 +1000
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> On Tue, 2018-05-01 at 00:55 +1000, Nicholas Piggin wrote:
> > The RAW console does not need writes to be atomic, so relax
> > opal_put_chars to be able to do partial writes, and implement an
> > _atomic variant which does not take a spinlock. This API is used
> > in xmon, so the less locking that is used, the better chance there
> > is that a crash can be debugged.
>
> Same comment I already had :-) "atomic" in Linux tends to mean
> something else (ie, atomic context), so I'd rather have something
> like opal_put_chars_sync() or such...
Oh yeah, I didn't ignore you, just... I thought atomic was okay.
atomic *also* tends to mean happens atomically. I think the in
atomic context meaning actually tends to be inatomic.
Sync I actually thought could be more easily confused with
synchronous vs asynchronous here.
>
> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> > arch/powerpc/include/asm/opal.h | 1 +
> > arch/powerpc/platforms/powernv/opal.c | 37 +++++++++++++++++++--------
> > drivers/tty/hvc/hvc_opal.c | 18 +++++++++----
> > 3 files changed, 41 insertions(+), 15 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
> > index bbff49fab0e5..5d7072411561 100644
> > --- a/arch/powerpc/include/asm/opal.h
> > +++ b/arch/powerpc/include/asm/opal.h
> > @@ -303,6 +303,7 @@ extern void opal_configure_cores(void);
> >
> > extern int opal_get_chars(uint32_t vtermno, char *buf, int count);
> > extern int opal_put_chars(uint32_t vtermno, const char *buf, int total_len);
> > +extern int opal_put_chars_atomic(uint32_t vtermno, const char *buf, int total_len);
> > extern int opal_flush_console(uint32_t vtermno);
> >
> > extern void hvc_opal_init_early(void);
> > diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
> > index 55d4b1983110..bcdb90ada938 100644
> > --- a/arch/powerpc/platforms/powernv/opal.c
> > +++ b/arch/powerpc/platforms/powernv/opal.c
> > @@ -344,9 +344,9 @@ int opal_get_chars(uint32_t vtermno, char *buf, int count)
> > return 0;
> > }
> >
> > -int opal_put_chars(uint32_t vtermno, const char *data, int total_len)
> > +static int __opal_put_chars(uint32_t vtermno, const char *data, int total_len, bool atomic)
> > {
> > - unsigned long flags;
> > + unsigned long flags = 0 /* shut up gcc */;
> > int written;
> > __be64 olen;
> > s64 rc;
> > @@ -354,11 +354,8 @@ int opal_put_chars(uint32_t vtermno, const char *data, int total_len)
> > if (!opal.entry)
> > return -ENODEV;
> >
> > - /* We want put_chars to be atomic to avoid mangling of hvsi
> > - * packets. To do that, we first test for room and return
> > - * -EAGAIN if there isn't enough.
> > - */
> > - spin_lock_irqsave(&opal_write_lock, flags);
> > + if (atomic)
> > + spin_lock_irqsave(&opal_write_lock, flags);
> > rc = opal_console_write_buffer_space(vtermno, &olen);
> > if (rc || be64_to_cpu(olen) < total_len) {
> > /* Closed -> drop characters */
> > @@ -391,14 +388,18 @@ int opal_put_chars(uint32_t vtermno, const char *data, int total_len)
> >
> > written = be64_to_cpu(olen);
> > if (written < total_len) {
> > - /* Should not happen */
> > - pr_warn("atomic console write returned partial len=%d written=%d\n", total_len, written);
> > + if (atomic) {
> > + /* Should not happen */
> > + pr_warn("atomic console write returned partial "
> > + "len=%d written=%d\n", total_len, written);
> > + }
> > if (!written)
> > written = -EAGAIN;
> > }
> >
> > out:
> > - spin_unlock_irqrestore(&opal_write_lock, flags);
> > + if (atomic)
> > + spin_unlock_irqrestore(&opal_write_lock, flags);
> >
> > /* In the -EAGAIN case, callers loop, so we have to flush the console
> > * here in case they have interrupts off (and we don't want to wait
> > @@ -412,6 +413,22 @@ int opal_put_chars(uint32_t vtermno, const char *data, int total_len)
> > return written;
> > }
> >
> > +int opal_put_chars(uint32_t vtermno, const char *data, int total_len)
> > +{
> > + return __opal_put_chars(vtermno, data, total_len, false);
> > +}
> > +
> > +/*
> > + * opal_put_chars_atomic will not perform partial-writes. Data will be
> > + * atomically written to the terminal or not at all. This is not strictly
> > + * true at the moment because console space can race with OPAL's console
> > + * writes.
> > + */
> > +int opal_put_chars_atomic(uint32_t vtermno, const char *data, int total_len)
> > +{
> > + return __opal_put_chars(vtermno, data, total_len, true);
> > +}
> > +
> > int opal_flush_console(uint32_t vtermno)
> > {
> > s64 rc;
> > diff --git a/drivers/tty/hvc/hvc_opal.c b/drivers/tty/hvc/hvc_opal.c
> > index af122ad7f06d..0a72f98ee082 100644
> > --- a/drivers/tty/hvc/hvc_opal.c
> > +++ b/drivers/tty/hvc/hvc_opal.c
> > @@ -183,9 +183,15 @@ static int hvc_opal_probe(struct platform_device *dev)
> > return -ENOMEM;
> > pv->proto = proto;
> > hvc_opal_privs[termno] = pv;
> > - if (proto == HV_PROTOCOL_HVSI)
> > - hvsilib_init(&pv->hvsi, opal_get_chars, opal_put_chars,
> > + if (proto == HV_PROTOCOL_HVSI) {
> > + /*
> > + * We want put_chars to be atomic to avoid mangling of
> > + * hvsi packets.
> > + */
> > + hvsilib_init(&pv->hvsi,
> > + opal_get_chars, opal_put_chars_atomic,
> > termno, 0);
> > + }
> >
> > /* Instanciate now to establish a mapping index==vtermno */
> > hvc_instantiate(termno, termno, ops);
> > @@ -376,8 +382,9 @@ void __init hvc_opal_init_early(void)
> > else if (of_device_is_compatible(stdout_node,"ibm,opal-console-hvsi")) {
> > hvc_opal_boot_priv.proto = HV_PROTOCOL_HVSI;
> > ops = &hvc_opal_hvsi_ops;
> > - hvsilib_init(&hvc_opal_boot_priv.hvsi, opal_get_chars,
> > - opal_put_chars, index, 1);
> > + hvsilib_init(&hvc_opal_boot_priv.hvsi,
> > + opal_get_chars, opal_put_chars_atomic,
> > + index, 1);
> > /* HVSI, perform the handshake now */
> > hvsilib_establish(&hvc_opal_boot_priv.hvsi);
> > pr_devel("hvc_opal: Found HVSI console\n");
> > @@ -409,7 +416,8 @@ void __init udbg_init_debug_opal_hvsi(void)
> > hvc_opal_privs[index] = &hvc_opal_boot_priv;
> > hvc_opal_boot_termno = index;
> > udbg_init_opal_common();
> > - hvsilib_init(&hvc_opal_boot_priv.hvsi, opal_get_chars, opal_put_chars,
> > + hvsilib_init(&hvc_opal_boot_priv.hvsi,
> > + opal_get_chars, opal_put_chars_atomic,
> > index, 1);
> > hvsilib_establish(&hvc_opal_boot_priv.hvsi);
> > }
^ permalink raw reply
* Re: [PATCH 08/15] powerpc/powernv: implement opal_put_chars_atomic
From: Benjamin Herrenschmidt @ 2018-05-01 9:48 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev
Cc: Greg Kroah-Hartman, Jiri Slaby, linux-kernel
In-Reply-To: <20180430145558.4308-9-npiggin@gmail.com>
On Tue, 2018-05-01 at 00:55 +1000, Nicholas Piggin wrote:
> The RAW console does not need writes to be atomic, so relax
> opal_put_chars to be able to do partial writes, and implement an
> _atomic variant which does not take a spinlock. This API is used
> in xmon, so the less locking that is used, the better chance there
> is that a crash can be debugged.
Same comment I already had :-) "atomic" in Linux tends to mean
something else (ie, atomic context), so I'd rather have something
like opal_put_chars_sync() or such...
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> arch/powerpc/include/asm/opal.h | 1 +
> arch/powerpc/platforms/powernv/opal.c | 37 +++++++++++++++++++--------
> drivers/tty/hvc/hvc_opal.c | 18 +++++++++----
> 3 files changed, 41 insertions(+), 15 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
> index bbff49fab0e5..5d7072411561 100644
> --- a/arch/powerpc/include/asm/opal.h
> +++ b/arch/powerpc/include/asm/opal.h
> @@ -303,6 +303,7 @@ extern void opal_configure_cores(void);
>
> extern int opal_get_chars(uint32_t vtermno, char *buf, int count);
> extern int opal_put_chars(uint32_t vtermno, const char *buf, int total_len);
> +extern int opal_put_chars_atomic(uint32_t vtermno, const char *buf, int total_len);
> extern int opal_flush_console(uint32_t vtermno);
>
> extern void hvc_opal_init_early(void);
> diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
> index 55d4b1983110..bcdb90ada938 100644
> --- a/arch/powerpc/platforms/powernv/opal.c
> +++ b/arch/powerpc/platforms/powernv/opal.c
> @@ -344,9 +344,9 @@ int opal_get_chars(uint32_t vtermno, char *buf, int count)
> return 0;
> }
>
> -int opal_put_chars(uint32_t vtermno, const char *data, int total_len)
> +static int __opal_put_chars(uint32_t vtermno, const char *data, int total_len, bool atomic)
> {
> - unsigned long flags;
> + unsigned long flags = 0 /* shut up gcc */;
> int written;
> __be64 olen;
> s64 rc;
> @@ -354,11 +354,8 @@ int opal_put_chars(uint32_t vtermno, const char *data, int total_len)
> if (!opal.entry)
> return -ENODEV;
>
> - /* We want put_chars to be atomic to avoid mangling of hvsi
> - * packets. To do that, we first test for room and return
> - * -EAGAIN if there isn't enough.
> - */
> - spin_lock_irqsave(&opal_write_lock, flags);
> + if (atomic)
> + spin_lock_irqsave(&opal_write_lock, flags);
> rc = opal_console_write_buffer_space(vtermno, &olen);
> if (rc || be64_to_cpu(olen) < total_len) {
> /* Closed -> drop characters */
> @@ -391,14 +388,18 @@ int opal_put_chars(uint32_t vtermno, const char *data, int total_len)
>
> written = be64_to_cpu(olen);
> if (written < total_len) {
> - /* Should not happen */
> - pr_warn("atomic console write returned partial len=%d written=%d\n", total_len, written);
> + if (atomic) {
> + /* Should not happen */
> + pr_warn("atomic console write returned partial "
> + "len=%d written=%d\n", total_len, written);
> + }
> if (!written)
> written = -EAGAIN;
> }
>
> out:
> - spin_unlock_irqrestore(&opal_write_lock, flags);
> + if (atomic)
> + spin_unlock_irqrestore(&opal_write_lock, flags);
>
> /* In the -EAGAIN case, callers loop, so we have to flush the console
> * here in case they have interrupts off (and we don't want to wait
> @@ -412,6 +413,22 @@ int opal_put_chars(uint32_t vtermno, const char *data, int total_len)
> return written;
> }
>
> +int opal_put_chars(uint32_t vtermno, const char *data, int total_len)
> +{
> + return __opal_put_chars(vtermno, data, total_len, false);
> +}
> +
> +/*
> + * opal_put_chars_atomic will not perform partial-writes. Data will be
> + * atomically written to the terminal or not at all. This is not strictly
> + * true at the moment because console space can race with OPAL's console
> + * writes.
> + */
> +int opal_put_chars_atomic(uint32_t vtermno, const char *data, int total_len)
> +{
> + return __opal_put_chars(vtermno, data, total_len, true);
> +}
> +
> int opal_flush_console(uint32_t vtermno)
> {
> s64 rc;
> diff --git a/drivers/tty/hvc/hvc_opal.c b/drivers/tty/hvc/hvc_opal.c
> index af122ad7f06d..0a72f98ee082 100644
> --- a/drivers/tty/hvc/hvc_opal.c
> +++ b/drivers/tty/hvc/hvc_opal.c
> @@ -183,9 +183,15 @@ static int hvc_opal_probe(struct platform_device *dev)
> return -ENOMEM;
> pv->proto = proto;
> hvc_opal_privs[termno] = pv;
> - if (proto == HV_PROTOCOL_HVSI)
> - hvsilib_init(&pv->hvsi, opal_get_chars, opal_put_chars,
> + if (proto == HV_PROTOCOL_HVSI) {
> + /*
> + * We want put_chars to be atomic to avoid mangling of
> + * hvsi packets.
> + */
> + hvsilib_init(&pv->hvsi,
> + opal_get_chars, opal_put_chars_atomic,
> termno, 0);
> + }
>
> /* Instanciate now to establish a mapping index==vtermno */
> hvc_instantiate(termno, termno, ops);
> @@ -376,8 +382,9 @@ void __init hvc_opal_init_early(void)
> else if (of_device_is_compatible(stdout_node,"ibm,opal-console-hvsi")) {
> hvc_opal_boot_priv.proto = HV_PROTOCOL_HVSI;
> ops = &hvc_opal_hvsi_ops;
> - hvsilib_init(&hvc_opal_boot_priv.hvsi, opal_get_chars,
> - opal_put_chars, index, 1);
> + hvsilib_init(&hvc_opal_boot_priv.hvsi,
> + opal_get_chars, opal_put_chars_atomic,
> + index, 1);
> /* HVSI, perform the handshake now */
> hvsilib_establish(&hvc_opal_boot_priv.hvsi);
> pr_devel("hvc_opal: Found HVSI console\n");
> @@ -409,7 +416,8 @@ void __init udbg_init_debug_opal_hvsi(void)
> hvc_opal_privs[index] = &hvc_opal_boot_priv;
> hvc_opal_boot_termno = index;
> udbg_init_opal_common();
> - hvsilib_init(&hvc_opal_boot_priv.hvsi, opal_get_chars, opal_put_chars,
> + hvsilib_init(&hvc_opal_boot_priv.hvsi,
> + opal_get_chars, opal_put_chars_atomic,
> index, 1);
> hvsilib_establish(&hvc_opal_boot_priv.hvsi);
> }
^ permalink raw reply
* Re: [PATCH v4 5/6] bus: fsl-mc: supoprt dma configure for devices on fsl-mc bus
From: kbuild test robot @ 2018-05-01 5:43 UTC (permalink / raw)
To: Nipun Gupta
Cc: kbuild-all, robin.murphy, will.deacon, robh+dt, robh,
mark.rutland, catalin.marinas, gregkh, hch, joro, m.szyprowski,
shawnguo, frowand.list, bhelgaas, iommu, linux-kernel, devicetree,
linux-arm-kernel, linuxppc-dev, linux-pci, bharat.bhushan,
stuyoder, laurentiu.tudor, leoyang.li, Nipun Gupta
In-Reply-To: <1525069641-8523-6-git-send-email-nipun.gupta@nxp.com>
[-- Attachment #1: Type: text/plain, Size: 2664 bytes --]
Hi Nipun,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v4.17-rc3 next-20180430]
[cannot apply to iommu/next glikely/devicetree/next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Nipun-Gupta/Support-for-fsl-mc-bus-and-its-devices-in-SMMU/20180501-125745
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
drivers/bus/fsl-mc/fsl-mc-bus.c: In function 'fsl_mc_dma_configure':
>> drivers/bus/fsl-mc/fsl-mc-bus.c:137:9: error: too many arguments to function 'of_dma_configure'
return of_dma_configure(dev, dma_dev->of_node, 0);
^~~~~~~~~~~~~~~~
In file included from drivers/bus/fsl-mc/fsl-mc-bus.c:13:0:
include/linux/of_device.h:58:5: note: declared here
int of_dma_configure(struct device *dev, struct device_node *np);
^~~~~~~~~~~~~~~~
drivers/bus/fsl-mc/fsl-mc-bus.c: At top level:
>> drivers/bus/fsl-mc/fsl-mc-bus.c:161:3: error: 'struct bus_type' has no member named 'dma_configure'
.dma_configure = fsl_mc_dma_configure,
^~~~~~~~~~~~~
vim +/of_dma_configure +137 drivers/bus/fsl-mc/fsl-mc-bus.c
129
130 static int fsl_mc_dma_configure(struct device *dev)
131 {
132 struct device *dma_dev = dev;
133
134 while (dev_is_fsl_mc(dma_dev))
135 dma_dev = dma_dev->parent;
136
> 137 return of_dma_configure(dev, dma_dev->of_node, 0);
138 }
139
140 static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
141 char *buf)
142 {
143 struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev);
144
145 return sprintf(buf, "fsl-mc:v%08Xd%s\n", mc_dev->obj_desc.vendor,
146 mc_dev->obj_desc.type);
147 }
148 static DEVICE_ATTR_RO(modalias);
149
150 static struct attribute *fsl_mc_dev_attrs[] = {
151 &dev_attr_modalias.attr,
152 NULL,
153 };
154
155 ATTRIBUTE_GROUPS(fsl_mc_dev);
156
157 struct bus_type fsl_mc_bus_type = {
158 .name = "fsl-mc",
159 .match = fsl_mc_bus_match,
160 .uevent = fsl_mc_bus_uevent,
> 161 .dma_configure = fsl_mc_dma_configure,
162 .dev_groups = fsl_mc_dev_groups,
163 };
164 EXPORT_SYMBOL_GPL(fsl_mc_bus_type);
165
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 63098 bytes --]
^ permalink raw reply
* [PATCH v2 RFC 1/1] KVM: PPC: Book3S HV: pack VCORE IDs to access full VCPU ID space
From: Sam Bobroff @ 2018-05-01 5:04 UTC (permalink / raw)
To: linuxppc-dev; +Cc: kvm, kvm-ppc, paulus, david, clg
From: Sam Bobroff <sam.bobroff@au1.ibm.com>
It is not currently possible to create the full number of possible
VCPUs (KVM_MAX_VCPUS) on Power9 with KVM-HV when the guest uses less
threads per core than it's core stride (or "VSMT mode"). This is
because the VCORE ID and XIVE offsets to grow beyond KVM_MAX_VCPUS
even though the VCPU ID is less than KVM_MAX_VCPU_ID.
To address this, "pack" the VCORE ID and XIVE offsets by using
knowledge of the way the VCPU IDs will be used when there are less
guest threads per core than the core stride. The primary thread of
each core will always be used first. Then, if the guest uses more than
one thread per core, these secondary threads will sequentially follow
the primary in each core.
So, the only way an ID above KVM_MAX_VCPUS can be seen, is if the
VCPUs are being spaced apart, so at least half of each core is empty
and IDs between KVM_MAX_VCPUS and (KVM_MAX_VCPUS * 2) can be mapped
into the second half of each core (4..7, in an 8-thread core).
Similarly, if IDs above KVM_MAX_VCPUS * 2 are seen, at least 3/4 of
each core is being left empty, and we can map down into the second and
third quarters of each core (2, 3 and 5, 6 in an 8-thread core).
Lastly, if IDs above KVM_MAX_VCPUS * 4 are seen, only the primary
threads are being used and 7/8 of the core is empty, allowing use of
the 1, 3, 5 and 7 thread slots.
(Strides less than 8 are handled similarly.)
This allows the VCORE ID or offset to be calculated quickly from the
VCPU ID or XIVE server numbers, without access to the VCPU structure.
Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
---
Hello everyone,
I've tested this on P8 and P9, in lots of combinations of host and guest
threading modes and it has been fine but it does feel like a "tricky"
approach, so I still feel somewhat wary about it.
I've posted it as an RFC because I have not tested it with guest native-XIVE,
and I suspect that it will take some work to support it.
====== v1 -> v2: ======
Patch 1/1: KVM: PPC: Book3S HV: pack VCORE IDs to access full VCPU ID space
* Corrected places in kvm/book3s_xive.c where IDs weren't packed.
* Because kvmppc_pack_vcpu_id() is only called on P9, there is no need to test "emul_smt_mode > 1", so remove it.
* Re-ordered block_offsets[] to be more ascending.
* Added more detailed description of the packing algorithm.
====== v1: ======
Patch 1/1: KVM: PPC: Book3S HV: pack VCORE IDs to access full VCPU ID space
arch/powerpc/include/asm/kvm_book3s.h | 44 +++++++++++++++++++++++++++++++++++
arch/powerpc/kvm/book3s_hv.c | 14 +++++++----
arch/powerpc/kvm/book3s_xive.c | 19 +++++++++------
3 files changed, 66 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h
index 376ae803b69c..a8d9d625e873 100644
--- a/arch/powerpc/include/asm/kvm_book3s.h
+++ b/arch/powerpc/include/asm/kvm_book3s.h
@@ -368,4 +368,48 @@ extern int kvmppc_h_logical_ci_store(struct kvm_vcpu *vcpu);
#define SPLIT_HACK_MASK 0xff000000
#define SPLIT_HACK_OFFS 0xfb000000
+/* Pack a VCPU ID from the [0..KVM_MAX_VCPU_ID) space down to the
+ * [0..KVM_MAX_VCPUS) space, while using knowledge of the guest's core stride
+ * (but not it's actual threading mode, which is not available) to avoid
+ * collisions.
+ *
+ * The implementation leaves VCPU IDs from the range [0..KVM_MAX_VCPUS) (block
+ * 0) unchanged: if the guest is filling each VCORE completely then it will be
+ * using consecutive IDs and it will fill the space without any packing.
+ *
+ * For higher VCPU IDs, the packed ID is based on the VCPU ID modulo
+ * KVM_MAX_VCPUS (effectively masking off the top bits) and then an offset is
+ * added to avoid collisions.
+ *
+ * VCPU IDs in the range [KVM_MAX_VCPUS..(KVM_MAX_VCPUS*2)) (block 1) are only
+ * possible if the guest is leaving at least 1/2 of each VCORE empty, so IDs
+ * can be safely packed into the second half of each VCORE by adding an offset
+ * of (stride / 2).
+ *
+ * Similarly, if VCPU IDs in the range [(KVM_MAX_VCPUS*2)..(KVM_MAX_VCPUS*4))
+ * (blocks 2 and 3) are seen, the guest must be leaving at least 3/4 of each
+ * VCORE empty so packed IDs can be offset by (stride / 4) and (stride * 3 / 4).
+ *
+ * Finally, VCPU IDs from blocks 5..7 will only be seen if the guest is using a
+ * stride of 8 and 1 thread per core so the remaining offsets of 1, 3, 5 and 7
+ * must be free to use.
+ *
+ * (The offsets for each block are stored in block_offsets[], indexed by the
+ * block number if the stride is 8. For cases where the guest's stride is less
+ * than 8, we can re-use the block_offsets array by multiplying the block
+ * number by (MAX_SMT_THREADS / stride) to reach the correct entry.)
+ */
+static inline u32 kvmppc_pack_vcpu_id(struct kvm *kvm, u32 id)
+{
+ const int block_offsets[MAX_SMT_THREADS] = {0, 4, 2, 6, 1, 3, 5, 7};
+ int stride = kvm->arch.emul_smt_mode;
+ int block = (id / KVM_MAX_VCPUS) * (MAX_SMT_THREADS / stride);
+ u32 packed_id;
+
+ BUG_ON(block >= MAX_SMT_THREADS);
+ packed_id = (id % KVM_MAX_VCPUS) + block_offsets[block];
+ BUG_ON(packed_id >= KVM_MAX_VCPUS);
+ return packed_id;
+}
+
#endif /* __ASM_KVM_BOOK3S_H__ */
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 9cb9448163c4..49165cc90051 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -1762,7 +1762,7 @@ static int threads_per_vcore(struct kvm *kvm)
return threads_per_subcore;
}
-static struct kvmppc_vcore *kvmppc_vcore_create(struct kvm *kvm, int core)
+static struct kvmppc_vcore *kvmppc_vcore_create(struct kvm *kvm, int id)
{
struct kvmppc_vcore *vcore;
@@ -1776,7 +1776,7 @@ static struct kvmppc_vcore *kvmppc_vcore_create(struct kvm *kvm, int core)
init_swait_queue_head(&vcore->wq);
vcore->preempt_tb = TB_NIL;
vcore->lpcr = kvm->arch.lpcr;
- vcore->first_vcpuid = core * kvm->arch.smt_mode;
+ vcore->first_vcpuid = id;
vcore->kvm = kvm;
INIT_LIST_HEAD(&vcore->preempt_list);
@@ -1992,12 +1992,18 @@ static struct kvm_vcpu *kvmppc_core_vcpu_create_hv(struct kvm *kvm,
mutex_lock(&kvm->lock);
vcore = NULL;
err = -EINVAL;
- core = id / kvm->arch.smt_mode;
+ if (cpu_has_feature(CPU_FTR_ARCH_300)) {
+ BUG_ON(kvm->arch.smt_mode != 1);
+ core = kvmppc_pack_vcpu_id(kvm, id);
+ } else {
+ core = id / kvm->arch.smt_mode;
+ }
if (core < KVM_MAX_VCORES) {
vcore = kvm->arch.vcores[core];
+ BUG_ON(cpu_has_feature(CPU_FTR_ARCH_300) && vcore);
if (!vcore) {
err = -ENOMEM;
- vcore = kvmppc_vcore_create(kvm, core);
+ vcore = kvmppc_vcore_create(kvm, id & ~(kvm->arch.smt_mode - 1));
kvm->arch.vcores[core] = vcore;
kvm->arch.online_vcores++;
}
diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
index f9818d7d3381..dbd5887daf4a 100644
--- a/arch/powerpc/kvm/book3s_xive.c
+++ b/arch/powerpc/kvm/book3s_xive.c
@@ -317,6 +317,11 @@ static int xive_select_target(struct kvm *kvm, u32 *server, u8 prio)
return -EBUSY;
}
+static u32 xive_vp(struct kvmppc_xive *xive, u32 server)
+{
+ return xive->vp_base + kvmppc_pack_vcpu_id(xive->kvm, server);
+}
+
static u8 xive_lock_and_mask(struct kvmppc_xive *xive,
struct kvmppc_xive_src_block *sb,
struct kvmppc_xive_irq_state *state)
@@ -362,7 +367,7 @@ static u8 xive_lock_and_mask(struct kvmppc_xive *xive,
*/
if (xd->flags & OPAL_XIVE_IRQ_MASK_VIA_FW) {
xive_native_configure_irq(hw_num,
- xive->vp_base + state->act_server,
+ xive_vp(xive, state->act_server),
MASKED, state->number);
/* set old_p so we can track if an H_EOI was done */
state->old_p = true;
@@ -418,7 +423,7 @@ static void xive_finish_unmask(struct kvmppc_xive *xive,
*/
if (xd->flags & OPAL_XIVE_IRQ_MASK_VIA_FW) {
xive_native_configure_irq(hw_num,
- xive->vp_base + state->act_server,
+ xive_vp(xive, state->act_server),
state->act_priority, state->number);
/* If an EOI is needed, do it here */
if (!state->old_p)
@@ -495,7 +500,7 @@ static int xive_target_interrupt(struct kvm *kvm,
kvmppc_xive_select_irq(state, &hw_num, NULL);
return xive_native_configure_irq(hw_num,
- xive->vp_base + server,
+ xive_vp(xive, server),
prio, state->number);
}
@@ -883,7 +888,7 @@ int kvmppc_xive_set_mapped(struct kvm *kvm, unsigned long guest_irq,
* which is fine for a never started interrupt.
*/
xive_native_configure_irq(hw_irq,
- xive->vp_base + state->act_server,
+ xive_vp(xive, state->act_server),
state->act_priority, state->number);
/*
@@ -959,7 +964,7 @@ int kvmppc_xive_clr_mapped(struct kvm *kvm, unsigned long guest_irq,
/* Reconfigure the IPI */
xive_native_configure_irq(state->ipi_number,
- xive->vp_base + state->act_server,
+ xive_vp(xive, state->act_server),
state->act_priority, state->number);
/*
@@ -1084,7 +1089,7 @@ int kvmppc_xive_connect_vcpu(struct kvm_device *dev,
pr_devel("Duplicate !\n");
return -EEXIST;
}
- if (cpu >= KVM_MAX_VCPUS) {
+ if (cpu >= KVM_MAX_VCPU_ID) {
pr_devel("Out of bounds !\n");
return -EINVAL;
}
@@ -1098,7 +1103,7 @@ int kvmppc_xive_connect_vcpu(struct kvm_device *dev,
xc->xive = xive;
xc->vcpu = vcpu;
xc->server_num = cpu;
- xc->vp_id = xive->vp_base + cpu;
+ xc->vp_id = xive_vp(xive, cpu);
xc->mfrr = 0xff;
xc->valid = true;
--
2.16.1.74.g9b0b1f47b
^ permalink raw reply related
* Re: [PATCH RFC 1/1] KVM: PPC: Book3S HV: pack VCORE IDs to access full VCPU ID space
From: Sam Bobroff @ 2018-05-01 4:52 UTC (permalink / raw)
To: David Gibson; +Cc: kvm-ppc, paulus, linuxppc-dev, Cédric Le Goater, kvm
In-Reply-To: <20180424034825.GN19804@umbus.fritz.box>
[-- Attachment #1: Type: text/plain, Size: 13208 bytes --]
On Tue, Apr 24, 2018 at 01:48:25PM +1000, David Gibson wrote:
> On Tue, Apr 24, 2018 at 01:19:15PM +1000, Sam Bobroff wrote:
> > On Mon, Apr 23, 2018 at 11:06:35AM +0200, Cédric Le Goater wrote:
> > > On 04/16/2018 06:09 AM, David Gibson wrote:
> > > > On Thu, Apr 12, 2018 at 05:02:06PM +1000, Sam Bobroff wrote:
> > > >> It is not currently possible to create the full number of possible
> > > >> VCPUs (KVM_MAX_VCPUS) on Power9 with KVM-HV when the guest uses less
> > > >> threads per core than it's core stride (or "VSMT mode"). This is
> > > >> because the VCORE ID and XIVE offsets to grow beyond KVM_MAX_VCPUS
> > > >> even though the VCPU ID is less than KVM_MAX_VCPU_ID.
> > > >>
> > > >> To address this, "pack" the VCORE ID and XIVE offsets by using
> > > >> knowledge of the way the VCPU IDs will be used when there are less
> > > >> guest threads per core than the core stride. The primary thread of
> > > >> each core will always be used first. Then, if the guest uses more than
> > > >> one thread per core, these secondary threads will sequentially follow
> > > >> the primary in each core.
> > > >>
> > > >> So, the only way an ID above KVM_MAX_VCPUS can be seen, is if the
> > > >> VCPUs are being spaced apart, so at least half of each core is empty
> > > >> and IDs between KVM_MAX_VCPUS and (KVM_MAX_VCPUS * 2) can be mapped
> > > >> into the second half of each core (4..7, in an 8-thread core).
> > > >>
> > > >> Similarly, if IDs above KVM_MAX_VCPUS * 2 are seen, at least 3/4 of
> > > >> each core is being left empty, and we can map down into the second and
> > > >> third quarters of each core (2, 3 and 5, 6 in an 8-thread core).
> > > >>
> > > >> Lastly, if IDs above KVM_MAX_VCPUS * 4 are seen, only the primary
> > > >> threads are being used and 7/8 of the core is empty, allowing use of
> > > >> the 1, 3, 5 and 7 thread slots.
> > > >>
> > > >> (Strides less than 8 are handled similarly.)
> > > >>
> > > >> This allows the VCORE ID or offset to be calculated quickly from the
> > > >> VCPU ID or XIVE server numbers, without access to the VCPU structure.
> > > >>
> > > >> Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
> > > >> ---
> > > >> Hello everyone,
> > > >>
> > > >> I've tested this on P8 and P9, in lots of combinations of host and guest
> > > >> threading modes and it has been fine but it does feel like a "tricky"
> > > >> approach, so I still feel somewhat wary about it.
> > >
> > > Have you done any migration ?
> >
> > No, but I will :-)
> >
> > > >> I've posted it as an RFC because I have not tested it with guest native-XIVE,
> > > >> and I suspect that it will take some work to support it.
> > >
> > > The KVM XIVE device will be different for XIVE exploitation mode, same structures
> > > though. I will send a patchset shortly.
> >
> > Great. This is probably where conflicts between the host and guest
> > numbers will show up. (See dwg's question below.)
> >
> > > >> arch/powerpc/include/asm/kvm_book3s.h | 19 +++++++++++++++++++
> > > >> arch/powerpc/kvm/book3s_hv.c | 14 ++++++++++----
> > > >> arch/powerpc/kvm/book3s_xive.c | 9 +++++++--
> > > >> 3 files changed, 36 insertions(+), 6 deletions(-)
> > > >>
> > > >> diff --git a/arch/powerpc/include/asm/kvm_book3s.h b/arch/powerpc/include/asm/kvm_book3s.h
> > > >> index 376ae803b69c..1295056d564a 100644
> > > >> --- a/arch/powerpc/include/asm/kvm_book3s.h
> > > >> +++ b/arch/powerpc/include/asm/kvm_book3s.h
> > > >> @@ -368,4 +368,23 @@ extern int kvmppc_h_logical_ci_store(struct kvm_vcpu *vcpu);
> > > >> #define SPLIT_HACK_MASK 0xff000000
> > > >> #define SPLIT_HACK_OFFS 0xfb000000
> > > >>
> > > >> +/* Pack a VCPU ID from the [0..KVM_MAX_VCPU_ID) space down to the
> > > >> + * [0..KVM_MAX_VCPUS) space, while using knowledge of the guest's core stride
> > > >> + * (but not it's actual threading mode, which is not available) to avoid
> > > >> + * collisions.
> > > >> + */
> > > >> +static inline u32 kvmppc_pack_vcpu_id(struct kvm *kvm, u32 id)
> > > >> +{
> > > >> + const int block_offsets[MAX_SMT_THREADS] = {0, 4, 2, 6, 1, 5, 3, 7};
> > > >
> > > > I'd suggest 1,3,5,7 at the end rather than 1,5,3,7 - accomplishes
> > > > roughly the same thing, but I think makes the pattern more obvious.
> >
> > OK.
> >
> > > >> + int stride = kvm->arch.emul_smt_mode > 1 ?
> > > >> + kvm->arch.emul_smt_mode : kvm->arch.smt_mode;
> > > >
> > > > AFAICT from BUG_ON()s etc. at the callsites, kvm->arch.smt_mode must
> > > > always be 1 when this is called, so the conditional here doesn't seem
> > > > useful.
> >
> > Ah yes, right. (That was an older version when I was thinking of using
> > it for P8 as well but that didn't seem to be a good idea.)
> >
> > > >> + int block = (id / KVM_MAX_VCPUS) * (MAX_SMT_THREADS / stride);
> > > >> + u32 packed_id;
> > > >> +
> > > >> + BUG_ON(block >= MAX_SMT_THREADS);
> > > >> + packed_id = (id % KVM_MAX_VCPUS) + block_offsets[block];
> > > >> + BUG_ON(packed_id >= KVM_MAX_VCPUS);
> > > >> + return packed_id;
> > > >> +}
> > > >
> > > > It took me a while to wrap my head around the packing function, but I
> > > > think I got there in the end. It's pretty clever.
> >
> > Thanks, I'll try to add a better description as well :-)
> >
> > > > One thing bothers me, though. This certainly packs things under
> > > > KVM_MAX_VCPUS, but not necessarily under the actual number of vcpus.
> > > > e.g. KVM_MAC_VCPUS==16, 8 vcpus total, stride 8, 2 vthreads/vcore (as
> > > > qemu sees it), gives both unpacked IDs (0, 1, 8, 9, 16, 17, 24, 25)
> > > > and packed ids of (0, 1, 8, 9, 4, 5, 12, 13) - leaving 2, 3, 6, 7
> > > > etc. unused.
> >
> > That's right. The property it provides is that all the numbers are under
> > KVM_MAX_VCPUS (which, see below, is the size of the fixed areas) not
> > that they are sequential.
> >
> > > > So again, the question is what exactly are these remapped IDs useful
> > > > for. If we're indexing into a bare array of structures of size
> > > > KVM_MAX_VCPUS then we're *already* wasting a bunch of space by having
> > > > more entries than vcpus. If we're indexing into something sparser,
> > > > then why is the remapping worthwhile?
> >
> > Well, here's my thinking:
> >
> > At the moment, kvm->vcores[] and xive->vp_base are both sized by NR_CPUS
> > (via KVM_MAX_VCPUS and KVM_MAX_VCORES which are both NR_CPUS). This is
> > enough space for the maximum number of VCPUs, and some space is wasted
> > when the guest uses less than this (but KVM doesn't know how many will
> > be created, so we can't do better easily). The problem is that the
> > indicies overflow before all of those VCPUs can be created, not that
> > more space is needed.
> >
> > We could fix the overflow by expanding these areas to KVM_MAX_VCPU_ID
> > but that will use 8x the space we use now, and we know that no more than
> > KVM_MAX_VCPUS will be used so all this new space is basically wasted.
> >
> > So remapping seems better if it will work. (Ben H. was strongly against
> > wasting more XIVE space if possible.)
>
> Hm, ok. Are the relevant arrays here per-VM, or global? Or some of both?
Per-VM. They are the kvm->vcores[] array and the blocks of memory
pointed to by xive->vp_base.
> > In short, remapping provides a way to allow the guest to create it's full set
> > of VCPUs without wasting any more space than we do currently, without
> > having to do something more complicated like tracking used IDs or adding
> > additional KVM CAPs.
> >
> > > >> +
> > > >> #endif /* __ASM_KVM_BOOK3S_H__ */
> > > >> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> > > >> index 9cb9448163c4..49165cc90051 100644
> > > >> --- a/arch/powerpc/kvm/book3s_hv.c
> > > >> +++ b/arch/powerpc/kvm/book3s_hv.c
> > > >> @@ -1762,7 +1762,7 @@ static int threads_per_vcore(struct kvm *kvm)
> > > >> return threads_per_subcore;
> > > >> }
> > > >>
> > > >> -static struct kvmppc_vcore *kvmppc_vcore_create(struct kvm *kvm, int core)
> > > >> +static struct kvmppc_vcore *kvmppc_vcore_create(struct kvm *kvm, int id)
> > > >> {
> > > >> struct kvmppc_vcore *vcore;
> > > >>
> > > >> @@ -1776,7 +1776,7 @@ static struct kvmppc_vcore *kvmppc_vcore_create(struct kvm *kvm, int core)
> > > >> init_swait_queue_head(&vcore->wq);
> > > >> vcore->preempt_tb = TB_NIL;
> > > >> vcore->lpcr = kvm->arch.lpcr;
> > > >> - vcore->first_vcpuid = core * kvm->arch.smt_mode;
> > > >> + vcore->first_vcpuid = id;
> > > >> vcore->kvm = kvm;
> > > >> INIT_LIST_HEAD(&vcore->preempt_list);
> > > >>
> > > >> @@ -1992,12 +1992,18 @@ static struct kvm_vcpu *kvmppc_core_vcpu_create_hv(struct kvm *kvm,
> > > >> mutex_lock(&kvm->lock);
> > > >> vcore = NULL;
> > > >> err = -EINVAL;
> > > >> - core = id / kvm->arch.smt_mode;
> > > >> + if (cpu_has_feature(CPU_FTR_ARCH_300)) {
> > > >> + BUG_ON(kvm->arch.smt_mode != 1);
> > > >> + core = kvmppc_pack_vcpu_id(kvm, id);
> > > >> + } else {
> > > >> + core = id / kvm->arch.smt_mode;
> > > >> + }
> > > >> if (core < KVM_MAX_VCORES) {
> > > >> vcore = kvm->arch.vcores[core];
> > > >> + BUG_ON(cpu_has_feature(CPU_FTR_ARCH_300) && vcore);
> > > >> if (!vcore) {
> > > >> err = -ENOMEM;
> > > >> - vcore = kvmppc_vcore_create(kvm, core);
> > > >> + vcore = kvmppc_vcore_create(kvm, id & ~(kvm->arch.smt_mode - 1));
> > > >> kvm->arch.vcores[core] = vcore;
> > > >> kvm->arch.online_vcores++;
> > > >> }
> > > >> diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
> > > >> index f9818d7d3381..681dfe12a5f3 100644
> > > >> --- a/arch/powerpc/kvm/book3s_xive.c
> > > >> +++ b/arch/powerpc/kvm/book3s_xive.c
> > > >> @@ -317,6 +317,11 @@ static int xive_select_target(struct kvm *kvm, u32 *server, u8 prio)
> > > >> return -EBUSY;
> > > >> }
> > > >>
> > > >> +static u32 xive_vp(struct kvmppc_xive *xive, u32 server)
> > > >> +{
> > > >> + return xive->vp_base + kvmppc_pack_vcpu_id(xive->kvm, server);
> > > >> +}
> > > >> +
> > > >
> > > > I'm finding the XIVE indexing really baffling. There are a bunch of
> > > > other places where the code uses (xive->vp_base + NUMBER) directly.
> >
> > Ugh, yes. It looks like I botched part of my final cleanup and all the
> > cases you saw in kvm/book3s_xive.c should have been replaced with a call to
> > xive_vp(). I'll fix it and sorry for the confusion.
>
> Ok.
>
> > > This links the QEMU vCPU server NUMBER to a XIVE virtual processor number
> > > in OPAL. So we need to check that all used NUMBERs are, first, consistent
> > > and then, in the correct range.
> >
> > Right. My approach was to allow XIVE to keep using server numbers that
> > are equal to VCPU IDs, and just pack down the ID before indexing into
> > the vp_base area.
> >
> > > > If those are host side references, I guess they don't need updates for
> > > > this.
> >
> > These are all guest side references.
> >
> > > > But if that's the case, then how does indexing into the same array
> > > > with both host and guest server numbers make sense?
> >
> > Right, it doesn't make sense to mix host and guest server numbers when
> > we're remapping only the guest ones, but in this case (without native
> > guest XIVE support) it's just guest ones.
>
> Right. Will this remapping be broken by guest-visible XIVE? That is
> for the guest visible XIVE are we going to need to expose un-remapped
> XIVE server IDs to the guest?
I'm not sure, I'll start looking at that next.
> > > yes. VPs are allocated with KVM_MAX_VCPUS :
> > >
> > > xive->vp_base = xive_native_alloc_vp_block(KVM_MAX_VCPUS);
> > >
> > > but
> > >
> > > #define KVM_MAX_VCPU_ID (threads_per_subcore * KVM_MAX_VCORES)
> > >
> > > WE would need to change the allocation of the VPs I guess.
> >
> > Yes, this is one of the structures that overflow if we don't pack the IDs.
> >
> > > >> static u8 xive_lock_and_mask(struct kvmppc_xive *xive,
> > > >> struct kvmppc_xive_src_block *sb,
> > > >> struct kvmppc_xive_irq_state *state)
> > > >> @@ -1084,7 +1089,7 @@ int kvmppc_xive_connect_vcpu(struct kvm_device *dev,
> > > >> pr_devel("Duplicate !\n");
> > > >> return -EEXIST;
> > > >> }
> > > >> - if (cpu >= KVM_MAX_VCPUS) {
> > > >> + if (cpu >= KVM_MAX_VCPU_ID) {>>
> > > >> pr_devel("Out of bounds !\n");
> > > >> return -EINVAL;
> > > >> }
> > > >> @@ -1098,7 +1103,7 @@ int kvmppc_xive_connect_vcpu(struct kvm_device *dev,
> > > >> xc->xive = xive;
> > > >> xc->vcpu = vcpu;
> > > >> xc->server_num = cpu;
> > > >> - xc->vp_id = xive->vp_base + cpu;
> > > >> + xc->vp_id = xive_vp(xive, cpu);
> > > >> xc->mfrr = 0xff;
> > > >> xc->valid = true;
> > > >>
> > > >
> > >
>
>
>
> --
> David Gibson | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
> | _way_ _around_!
> http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [Skiboot] [PATCH 1/2] SLW: Remove stop1_lite and stop0 stop states
From: Nicholas Piggin @ 2018-05-01 3:47 UTC (permalink / raw)
To: Akshay Adiga, linuxppc-dev; +Cc: skiboot, stewart
In-Reply-To: <1525079529-2284-1-git-send-email-akshay.adiga@linux.vnet.ibm.com>
On Mon, 30 Apr 2018 14:42:08 +0530
Akshay Adiga <akshay.adiga@linux.vnet.ibm.com> wrote:
> Powersaving for stop0_lite and stop1_lite is observed to be quite similar
> and both states resume without state loss. Using context_switch test [1]
> we observe that stop0_lite has slightly lower latency, hence removing
> stop1_lite.
>
> [1] linux/tools/testing/selftests/powerpc/benchmarks/context_switch.c
>
> Signed-off-by: Akshay Adiga <akshay.adiga@linux.vnet.ibm.com>
I'm okay for removing stop1_lite and stop2_lite -- SMT switching
is very latency critical. If we decide to actually start saving
real power then SMT should already have been switched.
So I would put stop1_lite and stop2_lite removal in the same patch.
Then what do we have? stop0_lite, stop0, stop1 for our fast idle
states.
I would be against removing stop0 if that is our fastest way to
release SMT resources, even if there is only a small advantage. Why
not remove stop1 instead?
We also need to better evaluate stop0_lite. How much advantage does
that have over snooze?
Thanks,
Nick
> ---
> hw/slw.c | 30 ------------------------------
> 1 file changed, 30 deletions(-)
>
> diff --git a/hw/slw.c b/hw/slw.c
> index 3f9abaa..edfc783 100644
> --- a/hw/slw.c
> +++ b/hw/slw.c
> @@ -521,36 +521,6 @@ static struct cpu_idle_states power9_cpu_idle_states[] = {
> | OPAL_PM_PSSCR_TR(3),
> .pm_ctrl_reg_mask = OPAL_PM_PSSCR_MASK },
> {
> - .name = "stop0",
> - .latency_ns = 2000,
> - .residency_ns = 20000,
> - .flags = 0*OPAL_PM_DEC_STOP \
> - | 0*OPAL_PM_TIMEBASE_STOP \
> - | 1*OPAL_PM_LOSE_USER_CONTEXT \
> - | 0*OPAL_PM_LOSE_HYP_CONTEXT \
> - | 0*OPAL_PM_LOSE_FULL_CONTEXT \
> - | 1*OPAL_PM_STOP_INST_FAST,
> - .pm_ctrl_reg_val = OPAL_PM_PSSCR_RL(0) \
> - | OPAL_PM_PSSCR_MTL(3) \
> - | OPAL_PM_PSSCR_TR(3) \
> - | OPAL_PM_PSSCR_ESL \
> - | OPAL_PM_PSSCR_EC,
> - .pm_ctrl_reg_mask = OPAL_PM_PSSCR_MASK },
> - {
> - .name = "stop1_lite", /* Enter stop1 with no state loss */
> - .latency_ns = 4900,
> - .residency_ns = 49000,
> - .flags = 0*OPAL_PM_DEC_STOP \
> - | 0*OPAL_PM_TIMEBASE_STOP \
> - | 0*OPAL_PM_LOSE_USER_CONTEXT \
> - | 0*OPAL_PM_LOSE_HYP_CONTEXT \
> - | 0*OPAL_PM_LOSE_FULL_CONTEXT \
> - | 1*OPAL_PM_STOP_INST_FAST,
> - .pm_ctrl_reg_val = OPAL_PM_PSSCR_RL(1) \
> - | OPAL_PM_PSSCR_MTL(3) \
> - | OPAL_PM_PSSCR_TR(3),
> - .pm_ctrl_reg_mask = OPAL_PM_PSSCR_MASK },
> - {
> .name = "stop1",
> .latency_ns = 5000,
> .residency_ns = 50000,
^ permalink raw reply
* [resend] Revert "powerpc/powernv: Increase memory block size to 1GB on radix"
From: Balbir Singh @ 2018-05-01 2:57 UTC (permalink / raw)
To: linuxppc-dev; +Cc: mpe, anton, Balbir Singh, Michael Neuling
This commit was a stop-gap to prevent crashes on hotunplug, caused by
the mismatch between the 1G mappings used for the linear mapping and the
memory block size. Those issues are now resolved because we split the
linear mapping at hotunplug time if necessary, as implemented in commit
4dd5f8a99e79 ("powerpc/mm/radix: Split linear mapping on hot-unplug").
Signed-off-by: Balbir Singh <bsingharora@gmail.com>
Signed-off-by: Michael Neuling <mikey@neuling.org>
---
Resend with a newer commit message grabbed from an email sent by mpe.
arch/powerpc/platforms/powernv/setup.c | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/setup.c b/arch/powerpc/platforms/powernv/setup.c
index ef8c9ce53a61..fa63d3fff14c 100644
--- a/arch/powerpc/platforms/powernv/setup.c
+++ b/arch/powerpc/platforms/powernv/setup.c
@@ -356,15 +356,7 @@ static void pnv_kexec_cpu_down(int crash_shutdown, int secondary)
#ifdef CONFIG_MEMORY_HOTPLUG_SPARSE
static unsigned long pnv_memory_block_size(void)
{
- /*
- * We map the kernel linear region with 1GB large pages on radix. For
- * memory hot unplug to work our memory block size must be at least
- * this size.
- */
- if (radix_enabled())
- return 1UL * 1024 * 1024 * 1024;
- else
- return 256UL * 1024 * 1024;
+ return 256UL * 1024 * 1024;
}
#endif
--
2.13.6
^ permalink raw reply related
* [PATCH] powerpc/watchdog: provide more data in watchdog messages
From: Nicholas Piggin @ 2018-05-01 2:22 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin
Provide timebase and timebase of last heartbeat in watchdog lockup
messages. Also provide a stack trace of when a CPU becomes un-stuck,
which can be useful -- it could be where irqs are re-enabled, so it
may be the end of the critical section which is responsible for the
latency.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
A lockup + unstuck event now looks like this (with irqtrace enabled):
watchdog: CPU 1 self-detected hard LOCKUP @ udelay+0x40/0x60
watchdog: CPU 1 TB:82611697355, last heartbeat TB:75431975757
Modules linked in:
irq event stamp: 2250184
hardirqs last enabled at (2250183): [<c000000000bb9158>] _raw_spin_unlock_irqrestore+0x58/0xd0
hardirqs last disabled at (2250184): [<c00000000000e76c>] kernel_init+0x7c/0x1b0
softirqs last enabled at (2243178): [<c000000000bba5d8>] __do_softirq+0x4d8/0x6f8
softirqs last disabled at (2243159): [<c0000000000f4138>] irq_exit+0x108/0x1a0
CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.17.0-rc3-00003-g7e2de68fdf39 #319
NIP: c000000000026d40 LR: c00000000000e77c CTR: 0000000000000000
REGS: c000000fffcebd80 TRAP: 0900 Not tainted (4.17.0-rc3-00003-g7e2de68fdf39)
MSR: 9000000002009033 <SF,HV,VEC,EE,ME,IR,DR,RI,LE> CR: 44000824 XER: 20040000
CFAR: c000000000026d4c SOFTE: 1
GPR00: c00000000000e77c c000000006a07dc0 c000000001306000 00000002625a0000
GPR04: c00000000000e76c 0000000000000038 8f5c28f5c28f5c29 c000000ff0e18cb0
GPR08: c0000000011a6000 00000001802d8dcf 00000011bbdcdf4c 7bc4830ddfa301bf
GPR12: 0000000000000000 c000000fffffe300 c00000000000e6f8 0000000000000000
GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR20: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR24: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR28: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
NIP [c000000000026d40] udelay+0x40/0x60
LR [c00000000000e77c] kernel_init+0x8c/0x1b0
Call Trace:
[c000000006a07dc0] [c00000000000e76c] kernel_init+0x7c/0x1b0 (unreliable)
[c000000006a07e30] [c00000000000b9b0] ret_from_kernel_thread+0x5c/0xac
Instruction dump:
3d22ffe9 e929d400 7c6349d2 7c210b78 7d4c42a6 7d2c42a6 7d2a4850 7fa34840
409d0020 60000000 60000000 60000000 <7d2c42a6> 7d2a4850 7fa34840 419dfff4
time 86406258554
watchdog: CPU 1 became unstuck TB:86406275034
irq event stamp: 2250201
hardirqs last enabled at (2250200): [<c000000000bb9218>] _raw_spin_unlock_irq+0x48/0x80
hardirqs last disabled at (2250201): [<c000000000189ff8>] vprintk_emit+0x98/0x5c0
softirqs last enabled at (2243178): [<c000000000bba5d8>] __do_softirq+0x4d8/0x6f8
softirqs last disabled at (2250191): [<c0000000000f4138>] irq_exit+0x108/0x1a0
CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.17.0-rc3-00003-g7e2de68fdf39 #319
NIP: c00000000000abdc LR: c0000000000186b8 CTR: 000000003003e65c
REGS: c000000006a07b10 TRAP: 0901 Not tainted (4.17.0-rc3-00003-g7e2de68fdf39)
MSR: 9000000002009033 <SF,HV,VEC,EE,ME,IR,DR,RI,LE> CR: 48000824 XER: 00000000
CFAR: c000000000026d4c SOFTE: 0
GPR00: c000000000018660 c000000006a07d90 c000000001306000 0000000000000900
GPR04: 0000000000000001 0000000000000038 8f5c28f5c28f5c29 c000000ff0e17970
GPR08: c000000001348d98 00000002002d8dee 00000011bbdcdf4c 7bc4830ddfa301bf
GPR12: 0000000000000000 c000000fffffe300
NIP [c00000000000abdc] replay_interrupt_return+0x0/0x4
LR [c0000000000186b8] arch_local_irq_restore+0xd8/0xf0
Call Trace:
[c000000006a07d90] [c000000000018660] arch_local_irq_restore+0x80/0xf0 (unreliable)
[c000000006a07dc0] [c00000000000e7ac] kernel_init+0xbc/0x1b0
[c000000006a07e30] [c00000000000b9b0] ret_from_kernel_thread+0x5c/0xac
Instruction dump:
7d200026 618c8000 2c030900 4182e518 2c030500 4182f140 2c030f00 4182f278
2c030a00 4182ff9c 2c030e60 4182eea8 <4e800020> 7c781b78 480003a5 480003bd
arch/powerpc/kernel/watchdog.c | 26 +++++++++++++++++++++-----
1 file changed, 21 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kernel/watchdog.c b/arch/powerpc/kernel/watchdog.c
index 6256dc3b0087..8a0f7d97f8d9 100644
--- a/arch/powerpc/kernel/watchdog.c
+++ b/arch/powerpc/kernel/watchdog.c
@@ -111,7 +111,11 @@ static inline void wd_smp_unlock(unsigned long *flags)
static void wd_lockup_ipi(struct pt_regs *regs)
{
- pr_emerg("CPU %d Hard LOCKUP\n", raw_smp_processor_id());
+ int cpu = raw_smp_processor_id();
+
+ pr_emerg("CPU %d Hard LOCKUP\n", cpu);
+ pr_emerg("CPU %d TB:%lld, last heartbeat TB:%lld\n",
+ cpu, get_tb(), per_cpu(wd_timer_tb, cpu));
print_modules();
print_irqtrace_events(current);
if (regs)
@@ -154,6 +158,8 @@ static void watchdog_smp_panic(int cpu, u64 tb)
pr_emerg("CPU %d detected hard LOCKUP on other CPUs %*pbl\n",
cpu, cpumask_pr_args(&wd_smp_cpus_pending));
+ pr_emerg("CPU %d TB:%lld, last SMP heartbeat TB:%lld\n",
+ cpu, tb, wd_smp_last_reset_tb);
if (!sysctl_hardlockup_all_cpu_backtrace) {
/*
@@ -194,10 +200,19 @@ static void wd_smp_clear_cpu_pending(int cpu, u64 tb)
{
if (!cpumask_test_cpu(cpu, &wd_smp_cpus_pending)) {
if (unlikely(cpumask_test_cpu(cpu, &wd_smp_cpus_stuck))) {
+ struct pt_regs *regs = get_irq_regs();
unsigned long flags;
- pr_emerg("CPU %d became unstuck\n", cpu);
wd_smp_lock(&flags);
+
+ pr_emerg("CPU %d became unstuck TB:%lld\n",
+ cpu, tb);
+ print_irqtrace_events(current);
+ if (regs)
+ show_regs(regs);
+ else
+ dump_stack();
+
cpumask_clear_cpu(cpu, &wd_smp_cpus_stuck);
wd_smp_unlock(&flags);
}
@@ -245,8 +260,6 @@ void soft_nmi_interrupt(struct pt_regs *regs)
tb = get_tb();
if (tb - per_cpu(wd_timer_tb, cpu) >= wd_panic_timeout_tb) {
- per_cpu(wd_timer_tb, cpu) = tb;
-
wd_smp_lock(&flags);
if (cpumask_test_cpu(cpu, &wd_smp_cpus_stuck)) {
wd_smp_unlock(&flags);
@@ -254,7 +267,10 @@ void soft_nmi_interrupt(struct pt_regs *regs)
}
set_cpu_stuck(cpu, tb);
- pr_emerg("CPU %d self-detected hard LOCKUP @ %pS\n", cpu, (void *)regs->nip);
+ pr_emerg("CPU %d self-detected hard LOCKUP @ %pS\n",
+ cpu, (void *)regs->nip);
+ pr_emerg("CPU %d TB:%lld, last heartbeat TB:%lld\n",
+ cpu, get_tb(), per_cpu(wd_timer_tb, cpu));
print_modules();
print_irqtrace_events(current);
show_regs(regs);
--
2.17.0
^ permalink raw reply related
* [PATCH 08/14] powerpc: Wire up restartable sequences system call
From: Mathieu Desnoyers @ 2018-04-30 22:44 UTC (permalink / raw)
To: Peter Zijlstra, Paul E . McKenney, Boqun Feng, Andy Lutomirski,
Dave Watson
Cc: linux-kernel, linux-api, Paul Turner, Andrew Morton, Russell King,
Thomas Gleixner, Ingo Molnar, H . Peter Anvin, Andrew Hunter,
Andi Kleen, Chris Lameter, Ben Maurer, Steven Rostedt,
Josh Triplett, Linus Torvalds, Catalin Marinas, Will Deacon,
Michael Kerrisk, Joel Fernandes, Mathieu Desnoyers,
Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
linuxppc-dev
In-Reply-To: <20180430224433.17407-1-mathieu.desnoyers@efficios.com>
From: Boqun Feng <boqun.feng@gmail.com>
Wire up the rseq system call on powerpc.
This provides an ABI improving the speed of a user-space getcpu
operation on powerpc by skipping the getcpu system call on the fast
path, as well as improving the speed of user-space operations on per-cpu
data compared to using load-reservation/store-conditional atomics.
TODO: wire up rseq_syscall() on return from system call. It is used with
CONFIG_DEBUG_RSEQ=y to ensure system calls are not issued within rseq critical
section
Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <paulus@samba.org>
CC: Michael Ellerman <mpe@ellerman.id.au>
CC: Peter Zijlstra <peterz@infradead.org>
CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
CC: linuxppc-dev@lists.ozlabs.org
---
arch/powerpc/include/asm/systbl.h | 1 +
arch/powerpc/include/asm/unistd.h | 2 +-
arch/powerpc/include/uapi/asm/unistd.h | 1 +
3 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/systbl.h b/arch/powerpc/include/asm/systbl.h
index d61f9c96d916..45d4d37495fd 100644
--- a/arch/powerpc/include/asm/systbl.h
+++ b/arch/powerpc/include/asm/systbl.h
@@ -392,3 +392,4 @@ SYSCALL(statx)
SYSCALL(pkey_alloc)
SYSCALL(pkey_free)
SYSCALL(pkey_mprotect)
+SYSCALL(rseq)
diff --git a/arch/powerpc/include/asm/unistd.h b/arch/powerpc/include/asm/unistd.h
index daf1ba97a00c..1e9708632dce 100644
--- a/arch/powerpc/include/asm/unistd.h
+++ b/arch/powerpc/include/asm/unistd.h
@@ -12,7 +12,7 @@
#include <uapi/asm/unistd.h>
-#define NR_syscalls 387
+#define NR_syscalls 388
#define __NR__exit __NR_exit
diff --git a/arch/powerpc/include/uapi/asm/unistd.h b/arch/powerpc/include/uapi/asm/unistd.h
index 389c36fd8299..ac5ba55066dd 100644
--- a/arch/powerpc/include/uapi/asm/unistd.h
+++ b/arch/powerpc/include/uapi/asm/unistd.h
@@ -398,5 +398,6 @@
#define __NR_pkey_alloc 384
#define __NR_pkey_free 385
#define __NR_pkey_mprotect 386
+#define __NR_rseq 387
#endif /* _UAPI_ASM_POWERPC_UNISTD_H_ */
--
2.11.0
^ permalink raw reply related
* [PATCH 07/14] powerpc: Add support for restartable sequences
From: Mathieu Desnoyers @ 2018-04-30 22:44 UTC (permalink / raw)
To: Peter Zijlstra, Paul E . McKenney, Boqun Feng, Andy Lutomirski,
Dave Watson
Cc: linux-kernel, linux-api, Paul Turner, Andrew Morton, Russell King,
Thomas Gleixner, Ingo Molnar, H . Peter Anvin, Andrew Hunter,
Andi Kleen, Chris Lameter, Ben Maurer, Steven Rostedt,
Josh Triplett, Linus Torvalds, Catalin Marinas, Will Deacon,
Michael Kerrisk, Joel Fernandes, Mathieu Desnoyers,
Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
linuxppc-dev
In-Reply-To: <20180430224433.17407-1-mathieu.desnoyers@efficios.com>
From: Boqun Feng <boqun.feng@gmail.com>
Call the rseq_handle_notify_resume() function on return to userspace if
TIF_NOTIFY_RESUME thread flag is set.
Perform fixup on the pre-signal when a signal is delivered on top of a
restartable sequence critical section.
Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <paulus@samba.org>
CC: Michael Ellerman <mpe@ellerman.id.au>
CC: Peter Zijlstra <peterz@infradead.org>
CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
CC: linuxppc-dev@lists.ozlabs.org
---
arch/powerpc/Kconfig | 1 +
arch/powerpc/kernel/signal.c | 3 +++
2 files changed, 4 insertions(+)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index c32a181a7cbb..ed21a777e8c6 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -223,6 +223,7 @@ config PPC
select HAVE_SYSCALL_TRACEPOINTS
select HAVE_VIRT_CPU_ACCOUNTING
select HAVE_IRQ_TIME_ACCOUNTING
+ select HAVE_RSEQ
select IRQ_DOMAIN
select IRQ_FORCED_THREADING
select MODULES_USE_ELF_RELA
diff --git a/arch/powerpc/kernel/signal.c b/arch/powerpc/kernel/signal.c
index 61db86ecd318..d3bb3aaaf5ac 100644
--- a/arch/powerpc/kernel/signal.c
+++ b/arch/powerpc/kernel/signal.c
@@ -133,6 +133,8 @@ static void do_signal(struct task_struct *tsk)
/* Re-enable the breakpoints for the signal stack */
thread_change_pc(tsk, tsk->thread.regs);
+ rseq_signal_deliver(tsk->thread.regs);
+
if (is32) {
if (ksig.ka.sa.sa_flags & SA_SIGINFO)
ret = handle_rt_signal32(&ksig, oldset, tsk);
@@ -164,6 +166,7 @@ void do_notify_resume(struct pt_regs *regs, unsigned long thread_info_flags)
if (thread_info_flags & _TIF_NOTIFY_RESUME) {
clear_thread_flag(TIF_NOTIFY_RESUME);
tracehook_notify_resume(regs);
+ rseq_handle_notify_resume(regs);
}
user_enter();
--
2.11.0
^ permalink raw reply related
* Re: [PATCH v4 2/6] iommu: of: make of_pci_map_rid() available for other devices too
From: Bjorn Helgaas @ 2018-04-30 20:56 UTC (permalink / raw)
To: Nipun Gupta
Cc: robin.murphy, will.deacon, robh+dt, robh, mark.rutland,
catalin.marinas, gregkh, hch, joro, m.szyprowski, shawnguo,
frowand.list, bhelgaas, iommu, linux-kernel, devicetree,
linux-arm-kernel, linuxppc-dev, linux-pci, bharat.bhushan,
stuyoder, laurentiu.tudor, leoyang.li
In-Reply-To: <1525069641-8523-3-git-send-email-nipun.gupta@nxp.com>
On Mon, Apr 30, 2018 at 11:57:17AM +0530, Nipun Gupta wrote:
> iommu-map property is also used by devices with fsl-mc. This
> patch moves the of_pci_map_rid to generic location, so that it
> can be used by other busses too.
>
> 'of_pci_map_rid' is renamed here to 'of_map_rid' and there is no
> functional change done in the API.
>
> Signed-off-by: Nipun Gupta <nipun.gupta@nxp.com>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
> drivers/iommu/of_iommu.c | 5 +--
> drivers/of/base.c | 102 +++++++++++++++++++++++++++++++++++++++++++++++
> drivers/of/irq.c | 5 +--
> drivers/pci/of.c | 101 ----------------------------------------------
> include/linux/of.h | 11 +++++
> include/linux/of_pci.h | 10 -----
> 6 files changed, 117 insertions(+), 117 deletions(-)
>
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index 5c36a8b..811e160 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -149,9 +149,8 @@ static int of_pci_iommu_init(struct pci_dev *pdev, u16 alias, void *data)
> struct of_phandle_args iommu_spec = { .args_count = 1 };
> int err;
>
> - err = of_pci_map_rid(info->np, alias, "iommu-map",
> - "iommu-map-mask", &iommu_spec.np,
> - iommu_spec.args);
> + err = of_map_rid(info->np, alias, "iommu-map", "iommu-map-mask",
> + &iommu_spec.np, iommu_spec.args);
> if (err)
> return err == -ENODEV ? NO_IOMMU : err;
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 848f549..c7aac81 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -1995,3 +1995,105 @@ int of_find_last_cache_level(unsigned int cpu)
>
> return cache_level;
> }
> +
> +/**
> + * of_map_rid - Translate a requester ID through a downstream mapping.
> + * @np: root complex device node.
> + * @rid: device requester ID to map.
> + * @map_name: property name of the map to use.
> + * @map_mask_name: optional property name of the mask to use.
> + * @target: optional pointer to a target device node.
> + * @id_out: optional pointer to receive the translated ID.
> + *
> + * Given a device requester ID, look up the appropriate implementation-defined
> + * platform ID and/or the target device which receives transactions on that
> + * ID, as per the "iommu-map" and "msi-map" bindings. Either of @target or
> + * @id_out may be NULL if only the other is required. If @target points to
> + * a non-NULL device node pointer, only entries targeting that node will be
> + * matched; if it points to a NULL value, it will receive the device node of
> + * the first matching target phandle, with a reference held.
> + *
> + * Return: 0 on success or a standard error code on failure.
> + */
> +int of_map_rid(struct device_node *np, u32 rid,
> + const char *map_name, const char *map_mask_name,
> + struct device_node **target, u32 *id_out)
> +{
> + u32 map_mask, masked_rid;
> + int map_len;
> + const __be32 *map = NULL;
> +
> + if (!np || !map_name || (!target && !id_out))
> + return -EINVAL;
> +
> + map = of_get_property(np, map_name, &map_len);
> + if (!map) {
> + if (target)
> + return -ENODEV;
> + /* Otherwise, no map implies no translation */
> + *id_out = rid;
> + return 0;
> + }
> +
> + if (!map_len || map_len % (4 * sizeof(*map))) {
> + pr_err("%pOF: Error: Bad %s length: %d\n", np,
> + map_name, map_len);
> + return -EINVAL;
> + }
> +
> + /* The default is to select all bits. */
> + map_mask = 0xffffffff;
> +
> + /*
> + * Can be overridden by "{iommu,msi}-map-mask" property.
> + * If of_property_read_u32() fails, the default is used.
> + */
> + if (map_mask_name)
> + of_property_read_u32(np, map_mask_name, &map_mask);
> +
> + masked_rid = map_mask & rid;
> + for ( ; map_len > 0; map_len -= 4 * sizeof(*map), map += 4) {
> + struct device_node *phandle_node;
> + u32 rid_base = be32_to_cpup(map + 0);
> + u32 phandle = be32_to_cpup(map + 1);
> + u32 out_base = be32_to_cpup(map + 2);
> + u32 rid_len = be32_to_cpup(map + 3);
> +
> + if (rid_base & ~map_mask) {
> + pr_err("%pOF: Invalid %s translation - %s-mask (0x%x) ignores rid-base (0x%x)\n",
> + np, map_name, map_name,
> + map_mask, rid_base);
> + return -EFAULT;
> + }
> +
> + if (masked_rid < rid_base || masked_rid >= rid_base + rid_len)
> + continue;
> +
> + phandle_node = of_find_node_by_phandle(phandle);
> + if (!phandle_node)
> + return -ENODEV;
> +
> + if (target) {
> + if (*target)
> + of_node_put(phandle_node);
> + else
> + *target = phandle_node;
> +
> + if (*target != phandle_node)
> + continue;
> + }
> +
> + if (id_out)
> + *id_out = masked_rid - rid_base + out_base;
> +
> + pr_debug("%pOF: %s, using mask %08x, rid-base: %08x, out-base: %08x, length: %08x, rid: %08x -> %08x\n",
> + np, map_name, map_mask, rid_base, out_base,
> + rid_len, rid, masked_rid - rid_base + out_base);
> + return 0;
> + }
> +
> + pr_err("%pOF: Invalid %s translation - no match for rid 0x%x on %pOF\n",
> + np, map_name, rid, target && *target ? *target : NULL);
> + return -EFAULT;
> +}
> +EXPORT_SYMBOL_GPL(of_map_rid);
> diff --git a/drivers/of/irq.c b/drivers/of/irq.c
> index 02ad93a..e1f6f39 100644
> --- a/drivers/of/irq.c
> +++ b/drivers/of/irq.c
> @@ -22,7 +22,6 @@
> #include <linux/module.h>
> #include <linux/of.h>
> #include <linux/of_irq.h>
> -#include <linux/of_pci.h>
> #include <linux/string.h>
> #include <linux/slab.h>
>
> @@ -588,8 +587,8 @@ static u32 __of_msi_map_rid(struct device *dev, struct device_node **np,
> * "msi-map" property.
> */
> for (parent_dev = dev; parent_dev; parent_dev = parent_dev->parent)
> - if (!of_pci_map_rid(parent_dev->of_node, rid_in, "msi-map",
> - "msi-map-mask", np, &rid_out))
> + if (!of_map_rid(parent_dev->of_node, rid_in, "msi-map",
> + "msi-map-mask", np, &rid_out))
> break;
> return rid_out;
> }
> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index a28355c..d2cebbe 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -362,107 +362,6 @@ int of_pci_get_host_bridge_resources(struct device_node *dev,
> EXPORT_SYMBOL_GPL(of_pci_get_host_bridge_resources);
> #endif /* CONFIG_OF_ADDRESS */
>
> -/**
> - * of_pci_map_rid - Translate a requester ID through a downstream mapping.
> - * @np: root complex device node.
> - * @rid: PCI requester ID to map.
> - * @map_name: property name of the map to use.
> - * @map_mask_name: optional property name of the mask to use.
> - * @target: optional pointer to a target device node.
> - * @id_out: optional pointer to receive the translated ID.
> - *
> - * Given a PCI requester ID, look up the appropriate implementation-defined
> - * platform ID and/or the target device which receives transactions on that
> - * ID, as per the "iommu-map" and "msi-map" bindings. Either of @target or
> - * @id_out may be NULL if only the other is required. If @target points to
> - * a non-NULL device node pointer, only entries targeting that node will be
> - * matched; if it points to a NULL value, it will receive the device node of
> - * the first matching target phandle, with a reference held.
> - *
> - * Return: 0 on success or a standard error code on failure.
> - */
> -int of_pci_map_rid(struct device_node *np, u32 rid,
> - const char *map_name, const char *map_mask_name,
> - struct device_node **target, u32 *id_out)
> -{
> - u32 map_mask, masked_rid;
> - int map_len;
> - const __be32 *map = NULL;
> -
> - if (!np || !map_name || (!target && !id_out))
> - return -EINVAL;
> -
> - map = of_get_property(np, map_name, &map_len);
> - if (!map) {
> - if (target)
> - return -ENODEV;
> - /* Otherwise, no map implies no translation */
> - *id_out = rid;
> - return 0;
> - }
> -
> - if (!map_len || map_len % (4 * sizeof(*map))) {
> - pr_err("%pOF: Error: Bad %s length: %d\n", np,
> - map_name, map_len);
> - return -EINVAL;
> - }
> -
> - /* The default is to select all bits. */
> - map_mask = 0xffffffff;
> -
> - /*
> - * Can be overridden by "{iommu,msi}-map-mask" property.
> - * If of_property_read_u32() fails, the default is used.
> - */
> - if (map_mask_name)
> - of_property_read_u32(np, map_mask_name, &map_mask);
> -
> - masked_rid = map_mask & rid;
> - for ( ; map_len > 0; map_len -= 4 * sizeof(*map), map += 4) {
> - struct device_node *phandle_node;
> - u32 rid_base = be32_to_cpup(map + 0);
> - u32 phandle = be32_to_cpup(map + 1);
> - u32 out_base = be32_to_cpup(map + 2);
> - u32 rid_len = be32_to_cpup(map + 3);
> -
> - if (rid_base & ~map_mask) {
> - pr_err("%pOF: Invalid %s translation - %s-mask (0x%x) ignores rid-base (0x%x)\n",
> - np, map_name, map_name,
> - map_mask, rid_base);
> - return -EFAULT;
> - }
> -
> - if (masked_rid < rid_base || masked_rid >= rid_base + rid_len)
> - continue;
> -
> - phandle_node = of_find_node_by_phandle(phandle);
> - if (!phandle_node)
> - return -ENODEV;
> -
> - if (target) {
> - if (*target)
> - of_node_put(phandle_node);
> - else
> - *target = phandle_node;
> -
> - if (*target != phandle_node)
> - continue;
> - }
> -
> - if (id_out)
> - *id_out = masked_rid - rid_base + out_base;
> -
> - pr_debug("%pOF: %s, using mask %08x, rid-base: %08x, out-base: %08x, length: %08x, rid: %08x -> %08x\n",
> - np, map_name, map_mask, rid_base, out_base,
> - rid_len, rid, masked_rid - rid_base + out_base);
> - return 0;
> - }
> -
> - pr_err("%pOF: Invalid %s translation - no match for rid 0x%x on %pOF\n",
> - np, map_name, rid, target && *target ? *target : NULL);
> - return -EFAULT;
> -}
> -
> #if IS_ENABLED(CONFIG_OF_IRQ)
> /**
> * of_irq_parse_pci - Resolve the interrupt for a PCI device
> diff --git a/include/linux/of.h b/include/linux/of.h
> index 4d25e4f..f4251c3 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -545,6 +545,10 @@ const __be32 *of_prop_next_u32(struct property *prop, const __be32 *cur,
>
> extern int of_cpu_node_to_id(struct device_node *np);
>
> +int of_map_rid(struct device_node *np, u32 rid,
> + const char *map_name, const char *map_mask_name,
> + struct device_node **target, u32 *id_out);
> +
> #else /* CONFIG_OF */
>
> static inline void of_core_init(void)
> @@ -931,6 +935,13 @@ static inline int of_cpu_node_to_id(struct device_node *np)
> return -ENODEV;
> }
>
> +static inline int of_map_rid(struct device_node *np, u32 rid,
> + const char *map_name, const char *map_mask_name,
> + struct device_node **target, u32 *id_out)
> +{
> + return -EINVAL;
> +}
> +
> #define of_match_ptr(_ptr) NULL
> #define of_match_node(_matches, _node) NULL
> #endif /* CONFIG_OF */
> diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
> index 091033a..a23b44a 100644
> --- a/include/linux/of_pci.h
> +++ b/include/linux/of_pci.h
> @@ -17,9 +17,6 @@ struct device_node *of_pci_find_child_device(struct device_node *parent,
> int of_get_pci_domain_nr(struct device_node *node);
> int of_pci_get_max_link_speed(struct device_node *node);
> void of_pci_check_probe_only(void);
> -int of_pci_map_rid(struct device_node *np, u32 rid,
> - const char *map_name, const char *map_mask_name,
> - struct device_node **target, u32 *id_out);
> #else
> static inline struct device_node *of_pci_find_child_device(struct device_node *parent,
> unsigned int devfn)
> @@ -44,13 +41,6 @@ static inline int of_pci_get_devfn(struct device_node *np)
> return -1;
> }
>
> -static inline int of_pci_map_rid(struct device_node *np, u32 rid,
> - const char *map_name, const char *map_mask_name,
> - struct device_node **target, u32 *id_out)
> -{
> - return -EINVAL;
> -}
> -
> static inline int
> of_pci_get_max_link_speed(struct device_node *node)
> {
> --
> 1.9.1
>
^ 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