Re: [PATCH 11/12] swiotlb: merge swiotlb-xen initialization into swiotlb

2022-03-04 Thread Stefano Stabellini
On Fri, 4 Mar 2022, Christoph Hellwig wrote:
> On Thu, Mar 03, 2022 at 02:49:29PM -0800, Stefano Stabellini wrote:
> > On Thu, 3 Mar 2022, Christoph Hellwig wrote:
> > > On Wed, Mar 02, 2022 at 05:25:10PM -0800, Stefano Stabellini wrote:
> > > > Thinking more about it we actually need to drop the xen_initial_domain()
> > > > check otherwise some cases won't be functional (Dom0 not 1:1 mapped, or
> > > > DomU 1:1 mapped).
> > > 
> > > Hmm, but that would be the case even before this series, right?
> > 
> > Before this series we only have the xen_swiotlb_detect() check in
> > xen_mm_init, we don't have a second xen_initial_domain() check.
> > 
> > The issue is that this series is adding one more xen_initial_domain()
> > check in xen_mm_init.
> 
> In current mainline xen_mm_init calls xen_swiotlb_init unconditionally.
> But xen_swiotlb_init then calls xen_swiotlb_fixup after allocating
> the memory, which in turn calls xen_create_contiguous_region.
> xen_create_contiguous_region fails with -EINVAL for the
> !xen_initial_domain() and thus caues xen_swiotlb_fixup and
> xen_swiotlb_init to unwind and return -EINVAL.
> 
> So as far as I can tell there is no change in behavior, but maybe I'm
> missing something subtle?

You are right.

The xen_initial_domain() check in xen_create_contiguous_region() is
wrong and we should get rid of it. It is a leftover from before the
xen_swiotlb_detect rework.

We could either remove it or change it into another xen_swiotlb_detect()
check.

Feel free to add the patch to your series or fold it with another patch
or rework it as you prefer. Thanks for spotting this!

---

arm/xen: don't check for xen_initial_domain() in xen_create_contiguous_region

It used to be that Linux enabled swiotlb-xen when running a dom0 on ARM.
Since f5079a9a2a31 "xen/arm: introduce XENFEAT_direct_mapped and
XENFEAT_not_direct_mapped", Linux detects whether to enable or disable
swiotlb-xen based on the new feature flags: XENFEAT_direct_mapped and
XENFEAT_not_direct_mapped.

However, there is still a leftover xen_initial_domain() check in
xen_create_contiguous_region. Remove the check as
xen_create_contiguous_region is only called by swiotlb-xen during
initialization. If xen_create_contiguous_region is called, we know Linux
is running 1:1 mapped so there is no need for additional checks.

Also update the in-code comment.

Signed-off-by: Stefano Stabellini 


diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
index a7e54a087b80..28c207060253 100644
--- a/arch/arm/xen/mm.c
+++ b/arch/arm/xen/mm.c
@@ -122,10 +122,7 @@ int xen_create_contiguous_region(phys_addr_t pstart, 
unsigned int order,
 unsigned int address_bits,
 dma_addr_t *dma_handle)
 {
-   if (!xen_initial_domain())
-   return -EINVAL;
-
-   /* we assume that dom0 is mapped 1:1 for now */
+   /* the domain is 1:1 mapped to use swiotlb-xen */
*dma_handle = pstart;
return 0;
 }


[PATCH] pkeys: Make pkey unsigned in arch_set_user_pkey_access()

2022-03-04 Thread ira . weiny
From: Ira Weiny 

The WARN_ON check in arch_set_user_pkey_access() in the x86 architecture
fails to check for an invalid negative value.

A simple check for less than 0 would fix this issue however, in the call
stack below arch_set_user_pkey_access() the pkey should never be
negative on any architecture.  It is always best to use correct types
when possible.  x86 only supports 16 keys while ppc supports 32, u8 is
therefore large enough for all current architectures and likely those in
the future.

Change the type of the pkey passed to arch_set_user_pkey_access() to u8.

To: Dave Hansen 
To: Michael Ellerman 
Cc: Aneesh Kumar K.V 
Signed-off-by: Ira Weiny 
---
 arch/powerpc/include/asm/pkeys.h | 4 ++--
 arch/powerpc/mm/book3s64/pkeys.c | 2 +-
 arch/x86/include/asm/pkeys.h | 4 ++--
 arch/x86/kernel/fpu/xstate.c | 2 +-
 include/linux/pkeys.h| 2 +-
 5 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/include/asm/pkeys.h b/arch/powerpc/include/asm/pkeys.h
index 59a2c7dbc78f..e70615a1da9b 100644
--- a/arch/powerpc/include/asm/pkeys.h
+++ b/arch/powerpc/include/asm/pkeys.h
@@ -143,9 +143,9 @@ static inline int arch_override_mprotect_pkey(struct 
vm_area_struct *vma,
return __arch_override_mprotect_pkey(vma, prot, pkey);
 }
 
-extern int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
+extern int __arch_set_user_pkey_access(struct task_struct *tsk, u8 pkey,
   unsigned long init_val);
-static inline int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
+static inline int arch_set_user_pkey_access(struct task_struct *tsk, u8 pkey,
unsigned long init_val)
 {
if (!mmu_has_feature(MMU_FTR_PKEY))
diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
index 753e62ba67af..c048467669df 100644
--- a/arch/powerpc/mm/book3s64/pkeys.c
+++ b/arch/powerpc/mm/book3s64/pkeys.c
@@ -333,7 +333,7 @@ static inline void init_iamr(int pkey, u8 init_bits)
  * Set the access rights in AMR IAMR and UAMOR registers for @pkey to that
  * specified in @init_val.
  */
-int __arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
+int __arch_set_user_pkey_access(struct task_struct *tsk, u8 pkey,
unsigned long init_val)
 {
u64 new_amr_bits = 0x0ul;
diff --git a/arch/x86/include/asm/pkeys.h b/arch/x86/include/asm/pkeys.h
index 5292e6dfe2a7..48efb81f6cc6 100644
--- a/arch/x86/include/asm/pkeys.h
+++ b/arch/x86/include/asm/pkeys.h
@@ -9,7 +9,7 @@
  */
 #define arch_max_pkey() (cpu_feature_enabled(X86_FEATURE_OSPKE) ? 16 : 1)
 
-extern int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
+extern int arch_set_user_pkey_access(struct task_struct *tsk, u8 pkey,
unsigned long init_val);
 
 static inline bool arch_pkeys_enabled(void)
@@ -115,7 +115,7 @@ int mm_pkey_free(struct mm_struct *mm, int pkey)
return 0;
 }
 
-extern int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
+extern int arch_set_user_pkey_access(struct task_struct *tsk, u8 pkey,
unsigned long init_val);
 
 static inline int vma_pkey(struct vm_area_struct *vma)
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 7c7824ae7862..db511bec57e5 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -1068,7 +1068,7 @@ void *get_xsave_addr(struct xregs_state *xsave, int 
xfeature_nr)
  * This will go out and modify PKRU register to set the access
  * rights for @pkey to @init_val.
  */
-int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
+int arch_set_user_pkey_access(struct task_struct *tsk, u8 pkey,
  unsigned long init_val)
 {
u32 old_pkru, new_pkru_bits = 0;
diff --git a/include/linux/pkeys.h b/include/linux/pkeys.h
index 86be8bf27b41..aa40ed2fb0fc 100644
--- a/include/linux/pkeys.h
+++ b/include/linux/pkeys.h
@@ -35,7 +35,7 @@ static inline int mm_pkey_free(struct mm_struct *mm, int pkey)
return -EINVAL;
 }
 
-static inline int arch_set_user_pkey_access(struct task_struct *tsk, int pkey,
+static inline int arch_set_user_pkey_access(struct task_struct *tsk, u8 pkey,
unsigned long init_val)
 {
return 0;
-- 
2.35.1



Re: [PATCH 11/12] swiotlb: merge swiotlb-xen initialization into swiotlb

2022-03-04 Thread Christoph Hellwig
On Fri, Mar 04, 2022 at 03:18:23PM -0500, Boris Ostrovsky wrote:
> This indeed allows dom0 to boot. Not sure I see where in the next patch this 
> would have been fixed?

I thought it did, but it doesn't.  In the meantime I've pushed out an
updated branch with this folded in to:

git://git.infradead.org/users/hch/misc.git swiotlb-init-cleanup

> (BTW, just noticed in iommu_setup() you set this variable to 1. Should be 
> 'true')

Thank, I'll fix this up.


Re: [PATCH 11/12] swiotlb: merge swiotlb-xen initialization into swiotlb

2022-03-04 Thread Boris Ostrovsky



On 3/4/22 12:43 PM, Christoph Hellwig wrote:

On Fri, Mar 04, 2022 at 12:36:17PM -0500, Boris Ostrovsky wrote:

I bisected it to "x86: remove the IOMMU table infrastructure" but haven't 
actually looked at the code yet.

That looks like the swiotlb buffer did not get initialized at all, but I
can't really explain why.

Can you stick in a printk and see if xen_swiotlb_init_early gets called
at all?



Actually, that's the only thing I did do so far and yes, it does get called.

So, specifically for "x86: remove the IOMMU table infrastructure" I
think we need the one-liner below so that swiotlb_exit doesn't get called
for the Xen case.  But that should have been fixed up by the next
patch already.



This indeed allows dom0 to boot. Not sure I see where in the next patch this 
would have been fixed?


(BTW, just noticed in iommu_setup() you set this variable to 1. Should be 
'true')


-boris



Re: [PATCH 10/12] swiotlb: add a SWIOTLB_ANY flag to lift the low memory restriction

2022-03-04 Thread Dongli Zhang
Hi Michael,

On 3/4/22 10:12 AM, Michael Kelley (LINUX) wrote:
> From: Christoph Hellwig  Sent: Tuesday, March 1, 2022 2:53 AM
>>
>> Power SVM wants to allocate a swiotlb buffer that is not restricted to low 
>> memory for
>> the trusted hypervisor scheme.  Consolidate the support for this into the 
>> swiotlb_init
>> interface by adding a new flag.
> 
> Hyper-V Isolated VMs want to do the same thing of not restricting the swiotlb
> buffer to low memory.  That's what Tianyu Lan's patch set[1] is proposing.
> Hyper-V synthetic devices have no DMA addressing limitations, and the
> likelihood of using a PCI pass-thru device with addressing limitations in an
> Isolated VM seems vanishingly small.
> 
> So could use of the SWIOTLB_ANY flag be generalized?  Let Hyper-V init
> code set the flag before swiotlb_init() is called.  Or provide a CONFIG
> variable that Hyper-V Isolated VMs could set.

I used to send 64-bit swiotlb, while at that time people thought it was the same
as Restricted DMA patchset.

https://lore.kernel.org/all/20210203233709.19819-1-dongli.zh...@oracle.com/

However, I do not think Restricted DMA patchset is going to supports 64-bit (or
high memory) DMA. Is this what you are looking for?

Dongli Zhang

> 
> Michael
> 
> [1] 
> https://urldefense.com/v3/__https://lore.kernel.org/lkml/20220209122302.213882-1-ltyker...@gmail.com/__;!!ACWV5N9M2RV99hQ!fUx4fMgdQIrqJDDy-pbv9xMeyHX0rC6iN8176LWjylI2_lsjy03gysm0-lAbV1Yb7_g$
>  
> 
>>
>> Signed-off-by: Christoph Hellwig 
>> ---
>>  arch/powerpc/include/asm/svm.h   |  4 
>>  arch/powerpc/include/asm/swiotlb.h   |  1 +
>>  arch/powerpc/kernel/dma-swiotlb.c|  1 +
>>  arch/powerpc/mm/mem.c|  5 +
>>  arch/powerpc/platforms/pseries/svm.c | 26 +-
>>  include/linux/swiotlb.h  |  1 +
>>  kernel/dma/swiotlb.c |  9 +++--
>>  7 files changed, 12 insertions(+), 35 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/svm.h b/arch/powerpc/include/asm/svm.h
>> index 7546402d796af..85580b30aba48 100644
>> --- a/arch/powerpc/include/asm/svm.h
>> +++ b/arch/powerpc/include/asm/svm.h
>> @@ -15,8 +15,6 @@ static inline bool is_secure_guest(void)
>>  return mfmsr() & MSR_S;
>>  }
>>
>> -void __init svm_swiotlb_init(void);
>> -
>>  void dtl_cache_ctor(void *addr);
>>  #define get_dtl_cache_ctor()(is_secure_guest() ? dtl_cache_ctor : 
>> NULL)
>>
>> @@ -27,8 +25,6 @@ static inline bool is_secure_guest(void)
>>  return false;
>>  }
>>
>> -static inline void svm_swiotlb_init(void) {}
>> -
>>  #define get_dtl_cache_ctor() NULL
>>
>>  #endif /* CONFIG_PPC_SVM */
>> diff --git a/arch/powerpc/include/asm/swiotlb.h
>> b/arch/powerpc/include/asm/swiotlb.h
>> index 3c1a1cd161286..4203b5e0a88ed 100644
>> --- a/arch/powerpc/include/asm/swiotlb.h
>> +++ b/arch/powerpc/include/asm/swiotlb.h
>> @@ -9,6 +9,7 @@
>>  #include 
>>
>>  extern unsigned int ppc_swiotlb_enable;
>> +extern unsigned int ppc_swiotlb_flags;
>>
>>  #ifdef CONFIG_SWIOTLB
>>  void swiotlb_detect_4g(void);
>> diff --git a/arch/powerpc/kernel/dma-swiotlb.c b/arch/powerpc/kernel/dma-
>> swiotlb.c
>> index fc7816126a401..ba256c37bcc0f 100644
>> --- a/arch/powerpc/kernel/dma-swiotlb.c
>> +++ b/arch/powerpc/kernel/dma-swiotlb.c
>> @@ -10,6 +10,7 @@
>>  #include 
>>
>>  unsigned int ppc_swiotlb_enable;
>> +unsigned int ppc_swiotlb_flags;
>>
>>  void __init swiotlb_detect_4g(void)
>>  {
>> diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c index
>> e1519e2edc656..a4d65418c30a9 100644
>> --- a/arch/powerpc/mm/mem.c
>> +++ b/arch/powerpc/mm/mem.c
>> @@ -249,10 +249,7 @@ void __init mem_init(void)
>>   * back to to-down.
>>   */
>>  memblock_set_bottom_up(true);
>> -if (is_secure_guest())
>> -svm_swiotlb_init();
>> -else
>> -swiotlb_init(ppc_swiotlb_enable, 0);
>> +swiotlb_init(ppc_swiotlb_enable, ppc_swiotlb_flags);
>>  #endif
>>
>>  high_memory = (void *) __va(max_low_pfn * PAGE_SIZE); diff --git
>> a/arch/powerpc/platforms/pseries/svm.c b/arch/powerpc/platforms/pseries/svm.c
>> index c5228f4969eb2..3b4045d508ec8 100644
>> --- a/arch/powerpc/platforms/pseries/svm.c
>> +++ b/arch/powerpc/platforms/pseries/svm.c
>> @@ -28,7 +28,7 @@ static int __init init_svm(void)
>>   * need to use the SWIOTLB buffer for DMA even if dma_capable() says
>>   * otherwise.
>>   */
>> -swiotlb_force = SWIOTLB_FORCE;
>> +ppc_swiotlb_flags |= SWIOTLB_ANY | SWIOTLB_FORCE;
>>
>>  /* Share the SWIOTLB buffer with the host. */
>>  swiotlb_update_mem_attributes();
>> @@ -37,30 +37,6 @@ static int __init init_svm(void)  }  
>> machine_early_initcall(pseries,
>> init_svm);
>>
>> -/*
>> - * Initialize SWIOTLB. Essentially the same as swiotlb_init(), except that 
>> it
>> - * can allocate the buffer anywhere in memory. Since the hypervisor doesn't 
>> have
>> - * any addressing limitation, we don't need to allocate it in low addresses.
>> - */
>> -void __init svm_swiotlb_

[RFC PATCH] KVM: PPC: Book3s HV: Allow setting GTSE for the nested guest

2022-03-04 Thread Fabiano Rosas
We're currently getting a Program Interrupt inside the nested guest
kernel when running with GTSE disabled in the nested hypervisor. We
allow any guest a cmdline override of GTSE for migration purposes. The
nested guest does not know it needs to use the option and tries to run
'tlbie' with LPCR_GTSE=0.

The details are a bit more intricate:

QEMU always sets GTSE=1 in OV5 even before calling KVM. At prom_init,
guests use the OV5 value to set MMU_FTR_GTSE. This setting can be
overridden by 'radix_hcall_invalidate=on' in the kernel cmdline. The
option itself depends on the availability of
FW_FEATURE_RPT_INVALIDATE, which it tied to QEMU's cap-rpt-invalidate
capability.

The MMU_FTR_GTSE flag leads guests to set PROC_TABLE_GTSE in their
process tables and after H_REGISTER_PROC_TBL, both QEMU and KVM will
set LPCR_GTSE=1 for that guest. Unless the guest uses the cmdline
override, in which case:

  MMU_FTR_GTSE=0 -> PROC_TABLE_GTSE=0 -> LPCR_GTSE=0

We don't allow the nested hypervisor to set some LPCR bits for its
nested guests, so if the nested HV has LPCR_GTSE=0, its nested guests
will also have LPCR_GTSE=0. But since the only thing that can really
flip GTSE is the cmdline override, if a nested guest runs without it,
then the sequence goes:

  MMU_FTR_GTSE=1 -> PROC_TABLE_GTSE=1 -> LPCR_GTSE=0.

With LPCR_GTSE=0 the HW will treat 'tlbie' as HV privileged.

This patch allows a nested HV to set LPCR_GTSE for its nested guests
so the LPCR setting will match what the nested guest sees in OV5.

Signed-off-by: Fabiano Rosas 
---
 arch/powerpc/kvm/book3s_hv_nested.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kvm/book3s_hv_nested.c 
b/arch/powerpc/kvm/book3s_hv_nested.c
index 9d373f8963ee..5b9008d89f90 100644
--- a/arch/powerpc/kvm/book3s_hv_nested.c
+++ b/arch/powerpc/kvm/book3s_hv_nested.c
@@ -262,7 +262,7 @@ static void load_l2_hv_regs(struct kvm_vcpu *vcpu,
 * Don't let L1 change LPCR bits for the L2 except these:
 */
mask = LPCR_DPFD | LPCR_ILE | LPCR_TC | LPCR_AIL | LPCR_LD |
-   LPCR_LPES | LPCR_MER;
+   LPCR_LPES | LPCR_MER | LPCR_GTSE;
 
/*
 * Additional filtering is required depending on hardware
-- 
2.34.1



RE: [PATCH 10/12] swiotlb: add a SWIOTLB_ANY flag to lift the low memory restriction

2022-03-04 Thread Michael Kelley (LINUX)
From: Christoph Hellwig  Sent: Tuesday, March 1, 2022 2:53 AM
> 
> Power SVM wants to allocate a swiotlb buffer that is not restricted to low 
> memory for
> the trusted hypervisor scheme.  Consolidate the support for this into the 
> swiotlb_init
> interface by adding a new flag.

Hyper-V Isolated VMs want to do the same thing of not restricting the swiotlb
buffer to low memory.  That's what Tianyu Lan's patch set[1] is proposing.
Hyper-V synthetic devices have no DMA addressing limitations, and the
likelihood of using a PCI pass-thru device with addressing limitations in an
Isolated VM seems vanishingly small.

So could use of the SWIOTLB_ANY flag be generalized?  Let Hyper-V init
code set the flag before swiotlb_init() is called.  Or provide a CONFIG
variable that Hyper-V Isolated VMs could set.

Michael

[1] https://lore.kernel.org/lkml/20220209122302.213882-1-ltyker...@gmail.com/

> 
> Signed-off-by: Christoph Hellwig 
> ---
>  arch/powerpc/include/asm/svm.h   |  4 
>  arch/powerpc/include/asm/swiotlb.h   |  1 +
>  arch/powerpc/kernel/dma-swiotlb.c|  1 +
>  arch/powerpc/mm/mem.c|  5 +
>  arch/powerpc/platforms/pseries/svm.c | 26 +-
>  include/linux/swiotlb.h  |  1 +
>  kernel/dma/swiotlb.c |  9 +++--
>  7 files changed, 12 insertions(+), 35 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/svm.h b/arch/powerpc/include/asm/svm.h
> index 7546402d796af..85580b30aba48 100644
> --- a/arch/powerpc/include/asm/svm.h
> +++ b/arch/powerpc/include/asm/svm.h
> @@ -15,8 +15,6 @@ static inline bool is_secure_guest(void)
>   return mfmsr() & MSR_S;
>  }
> 
> -void __init svm_swiotlb_init(void);
> -
>  void dtl_cache_ctor(void *addr);
>  #define get_dtl_cache_ctor() (is_secure_guest() ? dtl_cache_ctor : NULL)
> 
> @@ -27,8 +25,6 @@ static inline bool is_secure_guest(void)
>   return false;
>  }
> 
> -static inline void svm_swiotlb_init(void) {}
> -
>  #define get_dtl_cache_ctor() NULL
> 
>  #endif /* CONFIG_PPC_SVM */
> diff --git a/arch/powerpc/include/asm/swiotlb.h
> b/arch/powerpc/include/asm/swiotlb.h
> index 3c1a1cd161286..4203b5e0a88ed 100644
> --- a/arch/powerpc/include/asm/swiotlb.h
> +++ b/arch/powerpc/include/asm/swiotlb.h
> @@ -9,6 +9,7 @@
>  #include 
> 
>  extern unsigned int ppc_swiotlb_enable;
> +extern unsigned int ppc_swiotlb_flags;
> 
>  #ifdef CONFIG_SWIOTLB
>  void swiotlb_detect_4g(void);
> diff --git a/arch/powerpc/kernel/dma-swiotlb.c b/arch/powerpc/kernel/dma-
> swiotlb.c
> index fc7816126a401..ba256c37bcc0f 100644
> --- a/arch/powerpc/kernel/dma-swiotlb.c
> +++ b/arch/powerpc/kernel/dma-swiotlb.c
> @@ -10,6 +10,7 @@
>  #include 
> 
>  unsigned int ppc_swiotlb_enable;
> +unsigned int ppc_swiotlb_flags;
> 
>  void __init swiotlb_detect_4g(void)
>  {
> diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c index
> e1519e2edc656..a4d65418c30a9 100644
> --- a/arch/powerpc/mm/mem.c
> +++ b/arch/powerpc/mm/mem.c
> @@ -249,10 +249,7 @@ void __init mem_init(void)
>* back to to-down.
>*/
>   memblock_set_bottom_up(true);
> - if (is_secure_guest())
> - svm_swiotlb_init();
> - else
> - swiotlb_init(ppc_swiotlb_enable, 0);
> + swiotlb_init(ppc_swiotlb_enable, ppc_swiotlb_flags);
>  #endif
> 
>   high_memory = (void *) __va(max_low_pfn * PAGE_SIZE); diff --git
> a/arch/powerpc/platforms/pseries/svm.c b/arch/powerpc/platforms/pseries/svm.c
> index c5228f4969eb2..3b4045d508ec8 100644
> --- a/arch/powerpc/platforms/pseries/svm.c
> +++ b/arch/powerpc/platforms/pseries/svm.c
> @@ -28,7 +28,7 @@ static int __init init_svm(void)
>* need to use the SWIOTLB buffer for DMA even if dma_capable() says
>* otherwise.
>*/
> - swiotlb_force = SWIOTLB_FORCE;
> + ppc_swiotlb_flags |= SWIOTLB_ANY | SWIOTLB_FORCE;
> 
>   /* Share the SWIOTLB buffer with the host. */
>   swiotlb_update_mem_attributes();
> @@ -37,30 +37,6 @@ static int __init init_svm(void)  }  
> machine_early_initcall(pseries,
> init_svm);
> 
> -/*
> - * Initialize SWIOTLB. Essentially the same as swiotlb_init(), except that it
> - * can allocate the buffer anywhere in memory. Since the hypervisor doesn't 
> have
> - * any addressing limitation, we don't need to allocate it in low addresses.
> - */
> -void __init svm_swiotlb_init(void)
> -{
> - unsigned char *vstart;
> - unsigned long bytes, io_tlb_nslabs;
> -
> - io_tlb_nslabs = (swiotlb_size_or_default() >> IO_TLB_SHIFT);
> - io_tlb_nslabs = ALIGN(io_tlb_nslabs, IO_TLB_SEGSIZE);
> -
> - bytes = io_tlb_nslabs << IO_TLB_SHIFT;
> -
> - vstart = memblock_alloc(PAGE_ALIGN(bytes), PAGE_SIZE);
> - if (vstart && !swiotlb_init_with_tbl(vstart, io_tlb_nslabs, false))
> - return;
> -
> -
> - memblock_free(vstart, PAGE_ALIGN(io_tlb_nslabs << IO_TLB_SHIFT));
> - panic("SVM: Cannot allocate SWIOTLB buffer");
> -}
> -
>  int set_memory_encrypted(unsigned long addr, int nu

Re: [PATCH 11/12] swiotlb: merge swiotlb-xen initialization into swiotlb

2022-03-04 Thread Christoph Hellwig
On Fri, Mar 04, 2022 at 12:36:17PM -0500, Boris Ostrovsky wrote:
>>> I bisected it to "x86: remove the IOMMU table infrastructure" but haven't 
>>> actually looked at the code yet.
>> That looks like the swiotlb buffer did not get initialized at all, but I
>> can't really explain why.
>>
>> Can you stick in a printk and see if xen_swiotlb_init_early gets called
>> at all?
>
>
>
> Actually, that's the only thing I did do so far and yes, it does get called.

So, specifically for "x86: remove the IOMMU table infrastructure" I
think we need the one-liner below so that swiotlb_exit doesn't get called
for the Xen case.  But that should have been fixed up by the next
patch already.

diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
index 2ac0ef9c2fb76..1173aa282ab27 100644
--- a/arch/x86/kernel/pci-dma.c
+++ b/arch/x86/kernel/pci-dma.c
@@ -70,7 +70,7 @@ static void __init pci_xen_swiotlb_init(void)
if (!xen_initial_domain() && !x86_swiotlb_enable &&
swiotlb_force != SWIOTLB_FORCE)
return;
-   x86_swiotlb_enable = false;
+   x86_swiotlb_enable = true;
xen_swiotlb = true;
xen_swiotlb_init_early();
dma_ops = &xen_swiotlb_dma_ops;


Re: [PATCH 11/12] swiotlb: merge swiotlb-xen initialization into swiotlb

2022-03-04 Thread Boris Ostrovsky



On 3/4/22 12:28 PM, Christoph Hellwig wrote:

On Wed, Mar 02, 2022 at 08:15:03AM -0500, Boris Ostrovsky wrote:

Not for me, I fail to boot with

[   52.202000] bnxt_en :31:00.0: swiotlb buffer is full (sz: 256 bytes), 
total 0 (slots), used 0 (slots)

(this is iscsi root so I need the NIC).


I bisected it to "x86: remove the IOMMU table infrastructure" but haven't 
actually looked at the code yet.

That looks like the swiotlb buffer did not get initialized at all, but I
can't really explain why.

Can you stick in a printk and see if xen_swiotlb_init_early gets called
at all?




Actually, that's the only thing I did do so far and yes, it does get called.


-boris



Re: [PATCH 11/12] swiotlb: merge swiotlb-xen initialization into swiotlb

2022-03-04 Thread Christoph Hellwig
On Wed, Mar 02, 2022 at 08:15:03AM -0500, Boris Ostrovsky wrote:
> Not for me, I fail to boot with
>
> [   52.202000] bnxt_en :31:00.0: swiotlb buffer is full (sz: 256 bytes), 
> total 0 (slots), used 0 (slots)
>
> (this is iscsi root so I need the NIC).
>
>
> I bisected it to "x86: remove the IOMMU table infrastructure" but haven't 
> actually looked at the code yet.

That looks like the swiotlb buffer did not get initialized at all, but I
can't really explain why.

Can you stick in a printk and see if xen_swiotlb_init_early gets called
at all?


[PATCH v1 4/4] powerpc: Move C prototypes out of asm-prototypes.h

2022-03-04 Thread Christophe Leroy
We originally added asm-prototypes.h in commit 42f5b4cacd78 ("powerpc:
Introduce asm-prototypes.h"). It's purpose was for prototypes of C
functions that are only called from asm, in order to fix sparse
warnings about missing prototypes.

A few months later Nick added a different use case in
commit 4efca4ed05cb ("kbuild: modversions for EXPORT_SYMBOL() for asm")
for C prototypes for exported asm functions. This is basically the
inverse of our original usage.

Since then we've added various prototypes to asm-prototypes.h for both
reasons, meaning we now need to unstitch it all.

Dispatch prototypes of C functions into relevant headers and keep
only the prototypes for functions defined in assembly.

For the time being, leave prom_init() there because moving it
into asm/prom.h or asm/setup.h conflicts with
drivers/gpu/drm/nouveau/nvkm/subdev/bios/shadowrom.o
This will be fixed later by untaggling asm/pci.h and asm/prom.h
or by renaming the function in shadowrom.c

Signed-off-by: Christophe Leroy 
---
 arch/powerpc/include/asm/asm-prototypes.h | 51 ---
 arch/powerpc/include/asm/ftrace.h |  3 ++
 arch/powerpc/include/asm/hvcall.h |  5 ++
 arch/powerpc/include/asm/interrupt.h  | 11 
 arch/powerpc/include/asm/kexec.h  |  2 +
 arch/powerpc/include/asm/processor.h  |  8 +++
 arch/powerpc/include/asm/setup.h  |  7 +++
 arch/powerpc/include/asm/smp.h|  3 ++
 arch/powerpc/include/asm/syscalls.h   |  4 ++
 arch/powerpc/kernel/early_32.c|  1 -
 arch/powerpc/kernel/interrupt.c   |  1 -
 arch/powerpc/kernel/irq.c |  1 -
 arch/powerpc/kernel/mce.c |  1 -
 arch/powerpc/kernel/prom_init.c   |  1 -
 arch/powerpc/kernel/ptrace/ptrace.c   |  1 -
 arch/powerpc/kernel/setup_64.c|  1 -
 arch/powerpc/kernel/smp.c |  1 -
 arch/powerpc/kernel/syscalls.c|  1 -
 arch/powerpc/kernel/tau_6xx.c |  1 -
 arch/powerpc/kernel/time.c|  1 -
 arch/powerpc/kernel/trace/ftrace.c|  1 -
 arch/powerpc/kexec/core_64.c  |  1 -
 arch/powerpc/kvm/book3s_hv_builtin.c  |  1 -
 arch/powerpc/kvm/book3s_hv_rm_xive.c  |  1 -
 arch/powerpc/lib/vmx-helper.c |  1 -
 arch/powerpc/mm/book3s64/slb.c|  1 -
 arch/powerpc/mm/fault.c   |  1 -
 arch/powerpc/platforms/powernv/idle.c |  1 -
 .../platforms/powernv/opal-tracepoints.c  |  1 -
 arch/powerpc/platforms/pseries/lpar.c |  1 -
 30 files changed, 43 insertions(+), 72 deletions(-)

diff --git a/arch/powerpc/include/asm/asm-prototypes.h 
b/arch/powerpc/include/asm/asm-prototypes.h
index 83ef106923e6..65b7de62f7ec 100644
--- a/arch/powerpc/include/asm/asm-prototypes.h
+++ b/arch/powerpc/include/asm/asm-prototypes.h
@@ -19,19 +19,6 @@
 
 #include 
 
-/* SMP */
-extern struct task_struct *secondary_current;
-void start_secondary(void *unused);
-
-/* kexec */
-struct kimage;
-void kexec_copy_flush(struct kimage *image);
-
-/* pseries hcall tracing */
-extern struct static_key hcall_tracepoint_key;
-void __trace_hcall_entry(unsigned long opcode, unsigned long *args);
-void __trace_hcall_exit(long opcode, long retval, unsigned long *retbuf);
-
 /* Ultravisor */
 #if defined(CONFIG_PPC_POWERNV) || defined(CONFIG_PPC_SVM)
 long ucall_norets(unsigned long opcode, ...);
@@ -47,43 +34,12 @@ int64_t __opal_call(int64_t a0, int64_t a1, int64_t a2, 
int64_t a3,
int64_t a4, int64_t a5, int64_t a6, int64_t a7,
int64_t opcode, uint64_t msr);
 
-/* VMX copying */
-int enter_vmx_usercopy(void);
-int exit_vmx_usercopy(void);
-int enter_vmx_ops(void);
-void *exit_vmx_ops(void *dest);
-
-/* signals, syscalls and interrupts */
-#ifdef CONFIG_PPC32
-int
-ppc_select(int n, fd_set __user *inp, fd_set __user *outp, fd_set __user *exp,
-  struct __kernel_old_timeval __user *tvp);
-unsigned long __init early_init(unsigned long dt_ptr);
-void __init machine_init(u64 dt_ptr);
-#endif
-long system_call_exception(long r3, long r4, long r5, long r6, long r7, long 
r8, unsigned long r0, struct pt_regs *regs);
-notrace unsigned long syscall_exit_prepare(unsigned long r3, struct pt_regs 
*regs, long scv);
-notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs);
-notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs);
-#ifdef CONFIG_PPC64
-unsigned long syscall_exit_restart(unsigned long r3, struct pt_regs *regs);
-unsigned long interrupt_exit_user_restart(struct pt_regs *regs);
-unsigned long interrupt_exit_kernel_restart(struct pt_regs *regs);
-#endif
-
-long ppc_fadvise64_64(int fd, int advice, u32 offset_high, u32 offset_low,
- u32 len_high, u32 len_low);
-
 /* prom_init (OpenFirmware) */
 unsigned long __init prom_init(unsigned long r3, unsigned long r4

[PATCH v1 2/4] powerpc/smp: Declare current_set static

2022-03-04 Thread Christophe Leroy
current_set extern not needed anymore since
commit eafd825ed710 ("powerpc/64: Simplify __secondary_start
paca->kstack handling")

Signed-off-by: Christophe Leroy 
---
 arch/powerpc/include/asm/asm-prototypes.h | 1 -
 arch/powerpc/kernel/smp.c | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/asm-prototypes.h 
b/arch/powerpc/include/asm/asm-prototypes.h
index 4fd79207fd41..283bfeb8bf4c 100644
--- a/arch/powerpc/include/asm/asm-prototypes.h
+++ b/arch/powerpc/include/asm/asm-prototypes.h
@@ -20,7 +20,6 @@
 #include 
 
 /* SMP */
-extern struct task_struct *current_set[NR_CPUS];
 extern struct task_struct *secondary_current;
 void start_secondary(void *unused);
 
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index b7fd6a72aa76..7e30a6fe5adf 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -716,7 +716,7 @@ void smp_send_stop(void)
 }
 #endif /* CONFIG_NMI_IPI */
 
-struct task_struct *current_set[NR_CPUS];
+static struct task_struct *current_set[NR_CPUS];
 
 static void smp_store_cpu_info(int id)
 {
-- 
2.34.1



[PATCH v1 1/4] powerpc: Cleanup asm-prototypes.c

2022-03-04 Thread Christophe Leroy
Last call to sys_swapcontext() from ASM was removed by
commit fbcee2ebe8ed ("powerpc/32: Always save non volatile GPRs at
syscall entry")

sys_debug_setcontext() prototype not needed anymore since
commit f3675644e172 ("powerpc/syscalls: signal_{32, 64} - switch
to SYSCALL_DEFINE")

sys_switch_endian() prototype not needed anymore since
commit 81dac8177862 ("powerpc/64: Make sys_switch_endian() traceable")

_mount() prototype is already in asm/ftrace.h

Signed-off-by: Christophe Leroy 
---
 arch/powerpc/include/asm/asm-prototypes.h | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/arch/powerpc/include/asm/asm-prototypes.h 
b/arch/powerpc/include/asm/asm-prototypes.h
index 41b8a1e1144a..4fd79207fd41 100644
--- a/arch/powerpc/include/asm/asm-prototypes.h
+++ b/arch/powerpc/include/asm/asm-prototypes.h
@@ -57,12 +57,7 @@ int enter_vmx_ops(void);
 void *exit_vmx_ops(void *dest);
 
 /* signals, syscalls and interrupts */
-long sys_swapcontext(struct ucontext __user *old_ctx,
-   struct ucontext __user *new_ctx,
-   long ctx_size);
 #ifdef CONFIG_PPC32
-long sys_debug_setcontext(struct ucontext __user *ctx,
- int ndbg, struct sig_dbg_op __user *dbg);
 int
 ppc_select(int n, fd_set __user *inp, fd_set __user *outp, fd_set __user *exp,
   struct __kernel_old_timeval __user *tvp);
@@ -81,7 +76,6 @@ unsigned long interrupt_exit_kernel_restart(struct pt_regs 
*regs);
 
 long ppc_fadvise64_64(int fd, int advice, u32 offset_high, u32 offset_low,
  u32 len_high, u32 len_low);
-long sys_switch_endian(void);
 
 /* prom_init (OpenFirmware) */
 unsigned long __init prom_init(unsigned long r3, unsigned long r4,
@@ -102,7 +96,6 @@ extern int __cmpdi2(s64, s64);
 extern int __ucmpdi2(u64, u64);
 
 /* tracing */
-void _mcount(void);
 unsigned long prepare_ftrace_return(unsigned long parent, unsigned long ip,
unsigned long sp);
 
-- 
2.34.1



[PATCH v1 3/4] powerpc/kexec: Declare kexec_paca static

2022-03-04 Thread Christophe Leroy
kexec_paca is exclusively used in kexec/core_64.c

Declare is static.

Signed-off-by: Christophe Leroy 
---
 arch/powerpc/include/asm/asm-prototypes.h | 2 --
 arch/powerpc/kexec/core_64.c  | 2 +-
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/powerpc/include/asm/asm-prototypes.h 
b/arch/powerpc/include/asm/asm-prototypes.h
index 283bfeb8bf4c..83ef106923e6 100644
--- a/arch/powerpc/include/asm/asm-prototypes.h
+++ b/arch/powerpc/include/asm/asm-prototypes.h
@@ -24,9 +24,7 @@ extern struct task_struct *secondary_current;
 void start_secondary(void *unused);
 
 /* kexec */
-struct paca_struct;
 struct kimage;
-extern struct paca_struct kexec_paca;
 void kexec_copy_flush(struct kimage *image);
 
 /* pseries hcall tracing */
diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
index 635b5fc30b53..2d49dce129f2 100644
--- a/arch/powerpc/kexec/core_64.c
+++ b/arch/powerpc/kexec/core_64.c
@@ -291,7 +291,7 @@ static union thread_union kexec_stack __init_task_data =
  * For similar reasons to the stack above, the kexecing CPU needs to be on a
  * static PACA; we switch to kexec_paca.
  */
-struct paca_struct kexec_paca;
+static struct paca_struct kexec_paca;
 
 /* Our assembly helper, in misc_64.S */
 extern void kexec_sequence(void *newstack, unsigned long start,
-- 
2.34.1



Re: [PATCH 11/12] swiotlb: merge swiotlb-xen initialization into swiotlb

2022-03-04 Thread Christoph Hellwig
On Thu, Mar 03, 2022 at 02:49:29PM -0800, Stefano Stabellini wrote:
> On Thu, 3 Mar 2022, Christoph Hellwig wrote:
> > On Wed, Mar 02, 2022 at 05:25:10PM -0800, Stefano Stabellini wrote:
> > > Thinking more about it we actually need to drop the xen_initial_domain()
> > > check otherwise some cases won't be functional (Dom0 not 1:1 mapped, or
> > > DomU 1:1 mapped).
> > 
> > Hmm, but that would be the case even before this series, right?
> 
> Before this series we only have the xen_swiotlb_detect() check in
> xen_mm_init, we don't have a second xen_initial_domain() check.
> 
> The issue is that this series is adding one more xen_initial_domain()
> check in xen_mm_init.

In current mainline xen_mm_init calls xen_swiotlb_init unconditionally.
But xen_swiotlb_init then calls xen_swiotlb_fixup after allocating
the memory, which in turn calls xen_create_contiguous_region.
xen_create_contiguous_region fails with -EINVAL for the
!xen_initial_domain() and thus caues xen_swiotlb_fixup and
xen_swiotlb_init to unwind and return -EINVAL.

So as far as I can tell there is no change in behavior, but maybe I'm
missing something subtle?


Re: [PATCH] powerpc/sysdev: Use of_device_get_match_data()

2022-03-04 Thread Christophe Leroy


Le 04/03/2022 à 15:26, Marc Zyngier a écrit :
> On Fri, 04 Mar 2022 13:10:19 +,
> Christophe Leroy  wrote:
>>
>>
>>
>> Le 04/03/2022 à 02:18, cgel@gmail.com a écrit :
>>> From: Minghao Chi (CGEL ZTE) 
>>>
>>> Use of_device_get_match_data() to simplify the code.
>>>
>>> Reported-by: Zeal Robot 
>>> Signed-off-by: Minghao Chi (CGEL ZTE) 
>>> ---
>>>arch/powerpc/sysdev/fsl_msi.c | 6 +-
>>>1 file changed, 1 insertion(+), 5 deletions(-)
>>>
>>> diff --git a/arch/powerpc/sysdev/fsl_msi.c b/arch/powerpc/sysdev/fsl_msi.c
>>> index b3475ae9f236..9d135bbb30b7 100644
>>> --- a/arch/powerpc/sysdev/fsl_msi.c
>>> +++ b/arch/powerpc/sysdev/fsl_msi.c
>>> @@ -387,7 +387,6 @@ static int fsl_msi_setup_hwirq(struct fsl_msi *msi, 
>>> struct platform_device *dev,
>>>static const struct of_device_id fsl_of_msi_ids[];
>>>static int fsl_of_msi_probe(struct platform_device *dev)
>>>{
>>> -   const struct of_device_id *match;
>>> struct fsl_msi *msi;
>>> struct resource res, msiir;
>>> int err, i, j, irq_index, count;
>>> @@ -397,10 +396,7 @@ static int fsl_of_msi_probe(struct platform_device 
>>> *dev)
>>> u32 offset;
>>> struct pci_controller *phb;
>>>
>>> -   match = of_match_device(fsl_of_msi_ids, &dev->dev);
>>> -   if (!match)
>>> -   return -EINVAL;
>>> -   features = match->data;
>>> +   features = of_device_get_match_data(&dev->dev);
>>
>> What happens when features is NULL ?
> 
> I did jump at that one too, but as it turns out, it cannot happen, by
> construction. All the fsl_of_msi_ids[] entries have a non-NULL .data
> pointer, and you only enter probe if you match a fsl_of_msi_ids[]
> entry with the DT.
> 
> So the current check for a NULL match is not something that can happen
> short of some other bug somewhere.
> 

Ok.

Then it would be good to have a sentence explaining that in the commit 
message.

Christophe

Re: [PATCH] powerpc/sysdev: Use of_device_get_match_data()

2022-03-04 Thread Marc Zyngier
On Fri, 04 Mar 2022 13:10:19 +,
Christophe Leroy  wrote:
> 
> 
> 
> Le 04/03/2022 à 02:18, cgel@gmail.com a écrit :
> > From: Minghao Chi (CGEL ZTE) 
> > 
> > Use of_device_get_match_data() to simplify the code.
> > 
> > Reported-by: Zeal Robot 
> > Signed-off-by: Minghao Chi (CGEL ZTE) 
> > ---
> >   arch/powerpc/sysdev/fsl_msi.c | 6 +-
> >   1 file changed, 1 insertion(+), 5 deletions(-)
> > 
> > diff --git a/arch/powerpc/sysdev/fsl_msi.c b/arch/powerpc/sysdev/fsl_msi.c
> > index b3475ae9f236..9d135bbb30b7 100644
> > --- a/arch/powerpc/sysdev/fsl_msi.c
> > +++ b/arch/powerpc/sysdev/fsl_msi.c
> > @@ -387,7 +387,6 @@ static int fsl_msi_setup_hwirq(struct fsl_msi *msi, 
> > struct platform_device *dev,
> >   static const struct of_device_id fsl_of_msi_ids[];
> >   static int fsl_of_msi_probe(struct platform_device *dev)
> >   {
> > -   const struct of_device_id *match;
> > struct fsl_msi *msi;
> > struct resource res, msiir;
> > int err, i, j, irq_index, count;
> > @@ -397,10 +396,7 @@ static int fsl_of_msi_probe(struct platform_device 
> > *dev)
> > u32 offset;
> > struct pci_controller *phb;
> >   
> > -   match = of_match_device(fsl_of_msi_ids, &dev->dev);
> > -   if (!match)
> > -   return -EINVAL;
> > -   features = match->data;
> > +   features = of_device_get_match_data(&dev->dev);
> 
> What happens when features is NULL ?

I did jump at that one too, but as it turns out, it cannot happen, by
construction. All the fsl_of_msi_ids[] entries have a non-NULL .data
pointer, and you only enter probe if you match a fsl_of_msi_ids[]
entry with the DT.

So the current check for a NULL match is not something that can happen
short of some other bug somewhere.

M.

-- 
Without deviation from the norm, progress is not possible.


Re: [PATCH v2] powerpc: declare unmodified attribute_group usages const

2022-03-04 Thread Christophe Leroy


Le 04/03/2022 à 01:21, Rohan McLure a écrit :
> Inspired by (bd75b4ef4977: Constify static attribute_group structs),
> accepted by linux-next, reported:
> https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20220210202805.7750-4-rikard.falkeb...@gmail.com/
> 
> Nearly all singletons of type struct attribute_group are never
> modified, and so
> are candidates for being const. Declare them as const.
> 
> Signed-off-by: Rohan McLure 
> ---
>   arch/powerpc/perf/generic-compat-pmu.c  | 4 ++--
>   arch/powerpc/perf/hv-24x7.c | 6 +++---
>   arch/powerpc/perf/hv-gpci.c | 8 
>   arch/powerpc/perf/imc-pmu.c | 6 +++---
>   arch/powerpc/perf/isa207-common.c   | 2 +-
>   arch/powerpc/perf/power10-pmu.c | 6 +++---
>   arch/powerpc/perf/power7-pmu.c  | 4 ++--
>   arch/powerpc/perf/power8-pmu.c  | 4 ++--
>   arch/powerpc/perf/power9-pmu.c  | 6 +++---
>   arch/powerpc/platforms/cell/cbe_thermal.c   | 4 ++--
>   arch/powerpc/platforms/powernv/opal-core.c  | 2 +-
>   arch/powerpc/platforms/powernv/opal-dump.c  | 2 +-
>   arch/powerpc/platforms/powernv/opal-flash.c | 2 +-
>   arch/powerpc/platforms/pseries/papr_scm.c   | 2 +-
>   arch/powerpc/platforms/pseries/power.c  | 2 +-
>   15 files changed, 30 insertions(+), 30 deletions(-)
> 

/linux/arch/powerpc/platforms/cell/cbe_thermal.c: In function 
'thermal_init':
/linux/arch/powerpc/platforms/cell/cbe_thermal.c:370:26: error: passing 
argument 1 of 'spu_add_dev_attr_group' discards 'const' qualifier from 
pointer target type [-Werror=discarded-qualifiers]
spu_add_dev_attr_group(&spu_attribute_group);
   ^
In file included from /linux/arch/powerpc/platforms/cell/cbe_thermal.c:40:0:
/linux/arch/powerpc/include/asm/spu.h:252:5: note: expected 'struct 
attribute_group *' but argument is of type 'const struct attribute_group *'
  int spu_add_dev_attr_group(struct attribute_group *attrs);
  ^
/linux/arch/powerpc/platforms/cell/cbe_thermal.c:371:26: error: passing 
argument 1 of 'cpu_add_dev_attr_group' discards 'const' qualifier from 
pointer target type [-Werror=discarded-qualifiers]
cpu_add_dev_attr_group(&ppe_attribute_group);
   ^
In file included from /linux/arch/powerpc/platforms/cell/cbe_thermal.c:38:0:
/linux/include/linux/cpu.h:47:12: note: expected 'struct attribute_group 
*' but argument is of type 'const struct attribute_group *'
  extern int cpu_add_dev_attr_group(struct attribute_group *attrs);
 ^
/linux/arch/powerpc/platforms/cell/cbe_thermal.c: In function 
'thermal_exit':
/linux/arch/powerpc/platforms/cell/cbe_thermal.c:380:28: error: passing 
argument 1 of 'spu_remove_dev_attr_group' discards 'const' qualifier 
from pointer target type [-Werror=discarded-qualifiers]
   spu_remove_dev_attr_group(&spu_attribute_group);
 ^
In file included from /linux/arch/powerpc/platforms/cell/cbe_thermal.c:40:0:
/linux/arch/powerpc/include/asm/spu.h:253:6: note: expected 'struct 
attribute_group *' but argument is of type 'const struct attribute_group *'
  void spu_remove_dev_attr_group(struct attribute_group *attrs);
   ^
/linux/arch/powerpc/platforms/cell/cbe_thermal.c:381:28: error: passing 
argument 1 of 'cpu_remove_dev_attr_group' discards 'const' qualifier 
from pointer target type [-Werror=discarded-qualifiers]
   cpu_remove_dev_attr_group(&ppe_attribute_group);
 ^
In file included from /linux/arch/powerpc/platforms/cell/cbe_thermal.c:38:0:
/linux/include/linux/cpu.h:48:13: note: expected 'struct attribute_group 
*' but argument is of type 'const struct attribute_group *'
  extern void cpu_remove_dev_attr_group(struct attribute_group *attrs);
  ^
cc1: all warnings being treated as errors
make[4]: *** [arch/powerpc/platforms/cell/cbe_thermal.o] Error 1
/linux/scripts/Makefile.build:288: recipe for target 
'arch/powerpc/platforms/cell/cbe_thermal.o' failed

Re: [PATCH] powerpc/sysdev: Use of_device_get_match_data()

2022-03-04 Thread Christophe Leroy


Le 04/03/2022 à 02:18, cgel@gmail.com a écrit :
> From: Minghao Chi (CGEL ZTE) 
> 
> Use of_device_get_match_data() to simplify the code.
> 
> Reported-by: Zeal Robot 
> Signed-off-by: Minghao Chi (CGEL ZTE) 
> ---
>   arch/powerpc/sysdev/fsl_msi.c | 6 +-
>   1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/arch/powerpc/sysdev/fsl_msi.c b/arch/powerpc/sysdev/fsl_msi.c
> index b3475ae9f236..9d135bbb30b7 100644
> --- a/arch/powerpc/sysdev/fsl_msi.c
> +++ b/arch/powerpc/sysdev/fsl_msi.c
> @@ -387,7 +387,6 @@ static int fsl_msi_setup_hwirq(struct fsl_msi *msi, 
> struct platform_device *dev,
>   static const struct of_device_id fsl_of_msi_ids[];
>   static int fsl_of_msi_probe(struct platform_device *dev)
>   {
> - const struct of_device_id *match;
>   struct fsl_msi *msi;
>   struct resource res, msiir;
>   int err, i, j, irq_index, count;
> @@ -397,10 +396,7 @@ static int fsl_of_msi_probe(struct platform_device *dev)
>   u32 offset;
>   struct pci_controller *phb;
>   
> - match = of_match_device(fsl_of_msi_ids, &dev->dev);
> - if (!match)
> - return -EINVAL;
> - features = match->data;
> + features = of_device_get_match_data(&dev->dev);

What happens when features is NULL ?

>   
>   printk(KERN_DEBUG "Setting up Freescale MSI support\n");
>   

Re: [BUG] mtd: cfi_cmdset_0002: write regression since v4.17-rc1

2022-03-04 Thread Ahmad Fatoum
Hello Tokunori-san,

On 20.02.22 13:22, Tokunori Ikegami wrote:
> Hi Ahmad-san,
> 
> Could you please try the version 2 patch attached for the error case?
> This version is to check the DQ true data 0xFF by chip_good().

I had a similar patch locally as well at first. I just tested yours
and I can't reproduce the issue.

> But I am not sure if this works or not since the error is possible to be 
> caused by Hi-Z 0xff on floating bus or etc.

That it works for me could be because of Hi-Z 0xff, which is why
decided against it.

> What seems to work for me is checking if chip_good or chip_ready
> and map_word is equal to 0xFF. I can't justify why this is ok though.
> (Worst case bus is floating at this point of time and Hi-Z is read
> as 0xff on CPU data lines...)
 Sorry I am not sure about this.
 I thought the chip_ready() itself is correct as implemented as the data 
 sheet in the past.
 But it did not work correctly so changed to use chip_good() instead as it 
 is also correct.
>>> What exactly in the datasheet makes you believe chip_good is not 
>>> appropriate?
>> I just mentioned about the actual issue behaviors as not worked chip_good() 
>> on S29GL964N and not worked chip_ready() on MX29GL512FHT2I-11G before etc.
>> Anyway let me recheck the data sheet details as just checked it again 
>> quickly but needed more investigation to understand.
> 
> As far as I checked still both chip_good() and chip_ready() seem correct but 
> still the root cause is unknown.
> If as you mentioned the issue was cased by the DQ true data 0xFF I am not 
> sure why the read work without any error after the write operation.
> Also if the error was caused by the Hi-Z 0xff on floating bus as mentioned I 
> am not sure why the read work without any error after the write operation 
> with chip_ready().
> Sorry anyway the root cause is also unknown when the write operation was 
> changed to use chip_good() instead of chip_ready().

I've be ok with v1 then. Restores working behavior for me and shouldn't break 
others.

Cheers and thanks again,
Ahmad

> 
> Regards,
> Ikegami
> 
>>
>> Regards,
>> Ikegami
>>
>>>
>>> Cheers,
>>> Ahmad
>>>
>>>


-- 
Pengutronix e.K.   | |
Steuerwalder Str. 21   | http://www.pengutronix.de/  |
31137 Hildesheim, Germany  | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |


[Bug 215658] New: arch/powerpc/mm/mmu_context.o Assembler messages: Error: unrecognized opcode: `dssall' (PowerMac G4)

2022-03-04 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=215658

Bug ID: 215658
   Summary: arch/powerpc/mm/mmu_context.o Assembler messages:
Error: unrecognized opcode: `dssall' (PowerMac G4)
   Product: Platform Specific/Hardware
   Version: 2.5
Kernel Version: 5.16.12
  Hardware: PPC-32
OS: Linux
  Tree: Mainline
Status: NEW
  Severity: normal
  Priority: P1
 Component: PPC-32
  Assignee: platform_ppc...@kernel-bugs.osdl.org
  Reporter: erhar...@mailbox.org
Regression: Yes

Created attachment 300528
  --> https://bugzilla.kernel.org/attachment.cgi?id=300528&action=edit
kernel .config (5.16.12, PowerMac G4 DP)

5.16.12 kernel build for my G4 DP on my Talos II fails with:

[...]
  CC  arch/powerpc/mm/init_32.o
  CC  arch/powerpc/mm/pgtable_32.o
  CC  arch/powerpc/mm/pgtable-frag.o
  CC  arch/powerpc/mm/ioremap.o
  CC  arch/powerpc/mm/ioremap_32.o
  CC  arch/powerpc/mm/init-common.o
  CC  arch/powerpc/mm/mmu_context.o
{standard input}: Assembler messages:
{standard input}:30: Error: unrecognized opcode: `dssall'
make[2]: *** [scripts/Makefile.build:287: arch/powerpc/mm/mmu_context.o] Fehler
1
make[1]: *** [scripts/Makefile.build:549: arch/powerpc/mm] Fehler 2
make: *** [Makefile:1846: arch/powerpc] Error 2


This seems to have been introduced by commit
d51f86cfd8e378d4907958db77da3074f6dce3ba "powerpc/mm: Switch obsolete dssall to
.long"

Reverting this commit fixes the build for my G4.

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.