[PATCH v3 2/2] powerpc/pseries: increase timeout value for plpks_signed_update_var() H_CALL

2024-03-28 Thread Nayna Jain
Signed update H_CALL currently polls PHYP for 5msec. Update this to
5sec.

Signed-off-by: Nayna Jain 
Tested-by: Nageswara R Sastry 
---
v3:
 * Addition to Patch 1 timeout patch based on Andrew's feedback.

 arch/powerpc/platforms/pseries/plpks.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/plpks.c 
b/arch/powerpc/platforms/pseries/plpks.c
index bcfcd5acc5c2..4a595493d28a 100644
--- a/arch/powerpc/platforms/pseries/plpks.c
+++ b/arch/powerpc/platforms/pseries/plpks.c
@@ -463,9 +463,10 @@ int plpks_signed_update_var(struct plpks_var *var, u64 
flags)
 
continuetoken = retbuf[0];
if (pseries_status_to_err(rc) == -EBUSY) {
-   int delay_ms = get_longbusy_msecs(rc);
-   mdelay(delay_ms);
-   timeout += delay_ms;
+   int delay_us = get_longbusy_msecs(rc) * 1000;
+
+   fsleep(delay_us);
+   timeout += delay_us;
}
rc = pseries_status_to_err(rc);
} while (rc == -EBUSY && timeout < PLPKS_MAX_TIMEOUT);
-- 
2.31.1



[PATCH v3 1/2] powerpc/pseries: fix max polling time in plpks_confirm_object_flushed() function

2024-03-28 Thread Nayna Jain
usleep_range() function takes input time and range in usec. However,
currently it is assumed in msec in the function
plpks_confirm_object_flushed().

Fix the total polling time for the object flushing from 5msec to 5sec.

Reported-by: Nageswara R Sastry 
Fixes: 2454a7af0f2a ("powerpc/pseries: define driver for Platform KeyStore")
Signed-off-by: Nayna Jain 
Tested-by: Nageswara R Sastry 
---
v3:
No change

v2:
* Updated based on feedback from Michael Ellerman
Replaced usleep_range with fsleep.
Since there is no more need to specify range, sleep time is
reverted back to 10 msec.

 arch/powerpc/include/asm/plpks.h   | 5 ++---
 arch/powerpc/platforms/pseries/plpks.c | 3 +--
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/include/asm/plpks.h b/arch/powerpc/include/asm/plpks.h
index 23b77027c916..7a84069759b0 100644
--- a/arch/powerpc/include/asm/plpks.h
+++ b/arch/powerpc/include/asm/plpks.h
@@ -44,9 +44,8 @@
 #define PLPKS_MAX_DATA_SIZE4000
 
 // Timeouts for PLPKS operations
-#define PLPKS_MAX_TIMEOUT  5000 // msec
-#define PLPKS_FLUSH_SLEEP  10 // msec
-#define PLPKS_FLUSH_SLEEP_RANGE400
+#define PLPKS_MAX_TIMEOUT  (5 * USEC_PER_SEC)
+#define PLPKS_FLUSH_SLEEP  1 // usec
 
 struct plpks_var {
char *component;
diff --git a/arch/powerpc/platforms/pseries/plpks.c 
b/arch/powerpc/platforms/pseries/plpks.c
index febe18f251d0..bcfcd5acc5c2 100644
--- a/arch/powerpc/platforms/pseries/plpks.c
+++ b/arch/powerpc/platforms/pseries/plpks.c
@@ -415,8 +415,7 @@ static int plpks_confirm_object_flushed(struct label *label,
break;
}
 
-   usleep_range(PLPKS_FLUSH_SLEEP,
-PLPKS_FLUSH_SLEEP + PLPKS_FLUSH_SLEEP_RANGE);
+   fsleep(PLPKS_FLUSH_SLEEP);
timeout = timeout + PLPKS_FLUSH_SLEEP;
} while (timeout < PLPKS_MAX_TIMEOUT);
 
-- 
2.31.1



Re: [PATCH 2/9] dma: Convert from tasklet to BH workqueue

2024-03-28 Thread Allen
> >> > Since almost every driver associates the tasklet with the
> >> > dma_chan, we could go one step further and add the
> >> > work_queue structure directly into struct dma_chan,
> >> > with the wrapper operating on the dma_chan rather than
> >> > the work_queue.
> >>
> >> I think that is very great idea. having this wrapped in dma_chan would
> >> be very good way as well
> >>
> >> Am not sure if Allen is up for it :-)
> >
> >  Thanks Arnd, I know we did speak about this at LPC. I did start
> > working on using completion. I dropped it as I thought it would
> > be easier to move to workqueues.
>
> It's definitely easier to do the workqueue conversion as a first
> step, and I agree adding support for the completion right away is
> probably too much. Moving the work_struct into the dma_chan
> is probably not too hard though, if you leave your current
> approach for the cases where the tasklet is part of the
> dma_dev rather than the dma_chan.
>

 Alright, I will work on moving work_struck into the dma_chan and
leave the dma_dev as is (using bh workqueues) and post a RFC.
Once reviewed, I could move to the next step.

Thank you.

- Allen


Re: [PATCH 6/9] ipmi: Convert from tasklet to BH workqueue

2024-03-28 Thread Allen
> > > >
> > > >   Fair point. Could you please let me know once you have had a chance 
> > > > to test
> > > > these changes. Meanwhile, I will work on RFC wherein IPMI will have its 
> > > > own
> > > > workqueue.
> > > >
> > > >  Thanks for taking time out to review.
> > >
> > > After looking and thinking about it a bit, a BH context is still
> > > probably the best for this.
> > >
> > > I have tested this patch under load and various scenarios and it seems
> > > to work ok.  So:
> > >
> > > Tested-by: Corey Minyard 
> > > Acked-by: Corey Minyard 
> > >
> > > Or I can take this into my tree.
> > >
> > > -corey
> >
> >  Thank you very much. I think it should be okay for you to carry it into
> > your tree.
>
> Ok, it's in my for-next tree.  I fixed the directory reference, and I
> changed all the comments where you changed "tasklet" to "work" to
> instead say "workqueue".
>

 Thank you very much for fixing it.

- Allen


Re: [PATCH 6/9] ipmi: Convert from tasklet to BH workqueue

2024-03-28 Thread Corey Minyard
On Thu, Mar 28, 2024 at 12:41:22PM -0700, Allen wrote:
> > > > I believe that work queues items are execute single-threaded for a work
> > > > queue, so this should be good.  I need to test this, though.  It may be
> > > > that an IPMI device can have its own work queue; it may not be important
> > > > to run it in bh context.
> > >
> > >   Fair point. Could you please let me know once you have had a chance to 
> > > test
> > > these changes. Meanwhile, I will work on RFC wherein IPMI will have its 
> > > own
> > > workqueue.
> > >
> > >  Thanks for taking time out to review.
> >
> > After looking and thinking about it a bit, a BH context is still
> > probably the best for this.
> >
> > I have tested this patch under load and various scenarios and it seems
> > to work ok.  So:
> >
> > Tested-by: Corey Minyard 
> > Acked-by: Corey Minyard 
> >
> > Or I can take this into my tree.
> >
> > -corey
> 
>  Thank you very much. I think it should be okay for you to carry it into
> your tree.

Ok, it's in my for-next tree.  I fixed the directory reference, and I
changed all the comments where you changed "tasklet" to "work" to
instead say "workqueue".

-corey

> 
> - Allen
> 


Re: [PATCH 2/9] dma: Convert from tasklet to BH workqueue

2024-03-28 Thread Arnd Bergmann
On Thu, Mar 28, 2024, at 20:39, Allen wrote:
>> >
>> > Since almost every driver associates the tasklet with the
>> > dma_chan, we could go one step further and add the
>> > work_queue structure directly into struct dma_chan,
>> > with the wrapper operating on the dma_chan rather than
>> > the work_queue.
>>
>> I think that is very great idea. having this wrapped in dma_chan would
>> be very good way as well
>>
>> Am not sure if Allen is up for it :-)
>
>  Thanks Arnd, I know we did speak about this at LPC. I did start
> working on using completion. I dropped it as I thought it would
> be easier to move to workqueues.

It's definitely easier to do the workqueue conversion as a first
step, and I agree adding support for the completion right away is
probably too much. Moving the work_struct into the dma_chan
is probably not too hard though, if you leave your current
approach for the cases where the tasklet is part of the
dma_dev rather than the dma_chan.

  Arnd


Re: [PATCH 6/9] ipmi: Convert from tasklet to BH workqueue

2024-03-28 Thread Allen
> > > I believe that work queues items are execute single-threaded for a work
> > > queue, so this should be good.  I need to test this, though.  It may be
> > > that an IPMI device can have its own work queue; it may not be important
> > > to run it in bh context.
> >
> >   Fair point. Could you please let me know once you have had a chance to 
> > test
> > these changes. Meanwhile, I will work on RFC wherein IPMI will have its own
> > workqueue.
> >
> >  Thanks for taking time out to review.
>
> After looking and thinking about it a bit, a BH context is still
> probably the best for this.
>
> I have tested this patch under load and various scenarios and it seems
> to work ok.  So:
>
> Tested-by: Corey Minyard 
> Acked-by: Corey Minyard 
>
> Or I can take this into my tree.
>
> -corey

 Thank you very much. I think it should be okay for you to carry it into
your tree.

- Allen


Re: [PATCH 2/9] dma: Convert from tasklet to BH workqueue

2024-03-28 Thread Allen
> > >> The only generic interface to execute asynchronously in the BH context is
> > >> tasklet; however, it's marked deprecated and has some design flaws. To
> > >> replace tasklets, BH workqueue support was recently added. A BH workqueue
> > >> behaves similarly to regular workqueues except that the queued work items
> > >> are executed in the BH context.
> > >
> > > Thanks for conversion, am happy with BH alternative as it helps in
> > > dmaengine where we need shortest possible time between tasklet and
> > > interrupt handling to maximize dma performance
> >
> > I still feel that we want something different for dmaengine,
> > at least in the long run. As we have discussed in the past,
> > the tasklet context in these drivers is what the callbacks
> > from the dma client device is run in, and a lot of these probably
> > want something other than tasklet context, e.g. just call
> > complete() on a client-provided completion structure.
> >
> > Instead of open-coding the use of the system_bh_wq in each
> > dmaengine, how about we start with a custom WQ_BH
> > specifically for the dmaengine subsystem and wrap them
> > inside of another interface.
> >
> > Since almost every driver associates the tasklet with the
> > dma_chan, we could go one step further and add the
> > work_queue structure directly into struct dma_chan,
> > with the wrapper operating on the dma_chan rather than
> > the work_queue.
>
> I think that is very great idea. having this wrapped in dma_chan would
> be very good way as well
>
> Am not sure if Allen is up for it :-)

 Thanks Arnd, I know we did speak about this at LPC. I did start
working on using completion. I dropped it as I thought it would
be easier to move to workqueues.

Vinod, I would like to give this a shot and put out a RFC, I would
really appreciate review and feedback.

Thanks,
Allen

>
> --
> ~Vinod
>


Re: [PATCH 6/9] ipmi: Convert from tasklet to BH workqueue

2024-03-28 Thread Corey Minyard
On Thu, Mar 28, 2024 at 10:52:16AM -0700, Allen wrote:
> On Wed, Mar 27, 2024 at 11:05 AM Corey Minyard  wrote:
> >
> > I believe that work queues items are execute single-threaded for a work
> > queue, so this should be good.  I need to test this, though.  It may be
> > that an IPMI device can have its own work queue; it may not be important
> > to run it in bh context.
> 
>   Fair point. Could you please let me know once you have had a chance to test
> these changes. Meanwhile, I will work on RFC wherein IPMI will have its own
> workqueue.
> 
>  Thanks for taking time out to review.

After looking and thinking about it a bit, a BH context is still
probably the best for this.

I have tested this patch under load and various scenarios and it seems
to work ok.  So:

Tested-by: Corey Minyard 
Acked-by: Corey Minyard 

Or I can take this into my tree.

-corey

> 
> - Allen
> 
> >
> > -corey
> >
> > >
> > > Based on the work done by Tejun Heo 
> > > Branch: https://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git for-610
> > >
> > > Signed-off-by: Allen Pais 
> > > ---
> > >  drivers/char/ipmi/ipmi_msghandler.c | 30 ++---
> > >  1 file changed, 15 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/drivers/char/ipmi/ipmi_msghandler.c 
> > > b/drivers/char/ipmi/ipmi_msghandler.c
> > > index b0eedc4595b3..fce2a2dbdc82 100644
> > > --- a/drivers/char/ipmi/ipmi_msghandler.c
> > > +++ b/drivers/char/ipmi/ipmi_msghandler.c
> > > @@ -36,12 +36,13 @@
> > >  #include 
> > >  #include 
> > >  #include 
> > > +#include 
> > >
> > >  #define IPMI_DRIVER_VERSION "39.2"
> > >
> > >  static struct ipmi_recv_msg *ipmi_alloc_recv_msg(void);
> > >  static int ipmi_init_msghandler(void);
> > > -static void smi_recv_tasklet(struct tasklet_struct *t);
> > > +static void smi_recv_work(struct work_struct *t);
> > >  static void handle_new_recv_msgs(struct ipmi_smi *intf);
> > >  static void need_waiter(struct ipmi_smi *intf);
> > >  static int handle_one_recv_msg(struct ipmi_smi *intf,
> > > @@ -498,13 +499,13 @@ struct ipmi_smi {
> > >   /*
> > >* Messages queued for delivery.  If delivery fails (out of memory
> > >* for instance), They will stay in here to be processed later in a
> > > -  * periodic timer interrupt.  The tasklet is for handling received
> > > +  * periodic timer interrupt.  The work is for handling received
> > >* messages directly from the handler.
> > >*/
> > >   spinlock_t   waiting_rcv_msgs_lock;
> > >   struct list_head waiting_rcv_msgs;
> > >   atomic_t watchdog_pretimeouts_to_deliver;
> > > - struct tasklet_struct recv_tasklet;
> > > + struct work_struct recv_work;
> > >
> > >   spinlock_t xmit_msgs_lock;
> > >   struct list_head   xmit_msgs;
> > > @@ -704,7 +705,7 @@ static void clean_up_interface_data(struct ipmi_smi 
> > > *intf)
> > >   struct cmd_rcvr  *rcvr, *rcvr2;
> > >   struct list_head list;
> > >
> > > - tasklet_kill(>recv_tasklet);
> > > + cancel_work_sync(>recv_work);
> > >
> > >   free_smi_msg_list(>waiting_rcv_msgs);
> > >   free_recv_msg_list(>waiting_events);
> > > @@ -1319,7 +1320,7 @@ static void free_user(struct kref *ref)
> > >  {
> > >   struct ipmi_user *user = container_of(ref, struct ipmi_user, 
> > > refcount);
> > >
> > > - /* SRCU cleanup must happen in task context. */
> > > + /* SRCU cleanup must happen in work context. */
> > >   queue_work(remove_work_wq, >remove_work);
> > >  }
> > >
> > > @@ -3605,8 +3606,7 @@ int ipmi_add_smi(struct module *owner,
> > >   intf->curr_seq = 0;
> > >   spin_lock_init(>waiting_rcv_msgs_lock);
> > >   INIT_LIST_HEAD(>waiting_rcv_msgs);
> > > - tasklet_setup(>recv_tasklet,
> > > -  smi_recv_tasklet);
> > > + INIT_WORK(>recv_work, smi_recv_work);
> > >   atomic_set(>watchdog_pretimeouts_to_deliver, 0);
> > >   spin_lock_init(>xmit_msgs_lock);
> > >   INIT_LIST_HEAD(>xmit_msgs);
> > > @@ -4779,7 +4779,7 @@ static void handle_new_recv_msgs(struct ipmi_smi 
> > > *intf)
> > >* To preserve message order, quit if we
> > >* can't handle a message.  Add the message
> > >* back at the head, this is safe because this
> > > -  * tasklet is the only thing that pulls the
> > > +  * work is the only thing that pulls the
> > >* messages.
> > >*/
> > >   list_add(_msg->link, >waiting_rcv_msgs);
> > > @@ -4812,10 +4812,10 @@ static void handle_new_recv_msgs(struct ipmi_smi 
> > > *intf)
> > >   }
> > >  }
> > >
> > > -static void smi_recv_tasklet(struct tasklet_struct *t)
> > > +static void smi_recv_work(struct work_struct *t)
> > >  {
> > >   unsigned long flags = 0; /* keep us warning-free. */
> > > - struct ipmi_smi *intf = from_tasklet(intf, t, 

Re: [PATCH 2/9] dma: Convert from tasklet to BH workqueue

2024-03-28 Thread Vinod Koul
On 28-03-24, 11:08, Arnd Bergmann wrote:
> On Thu, Mar 28, 2024, at 06:55, Vinod Koul wrote:
> > On 27-03-24, 16:03, Allen Pais wrote:
> >> The only generic interface to execute asynchronously in the BH context is
> >> tasklet; however, it's marked deprecated and has some design flaws. To
> >> replace tasklets, BH workqueue support was recently added. A BH workqueue
> >> behaves similarly to regular workqueues except that the queued work items
> >> are executed in the BH context.
> >
> > Thanks for conversion, am happy with BH alternative as it helps in
> > dmaengine where we need shortest possible time between tasklet and
> > interrupt handling to maximize dma performance
> 
> I still feel that we want something different for dmaengine,
> at least in the long run. As we have discussed in the past,
> the tasklet context in these drivers is what the callbacks
> from the dma client device is run in, and a lot of these probably
> want something other than tasklet context, e.g. just call
> complete() on a client-provided completion structure.
> 
> Instead of open-coding the use of the system_bh_wq in each
> dmaengine, how about we start with a custom WQ_BH
> specifically for the dmaengine subsystem and wrap them
> inside of another interface.
> 
> Since almost every driver associates the tasklet with the
> dma_chan, we could go one step further and add the
> work_queue structure directly into struct dma_chan,
> with the wrapper operating on the dma_chan rather than
> the work_queue.

I think that is very great idea. having this wrapped in dma_chan would
be very good way as well

Am not sure if Allen is up for it :-)

-- 
~Vinod


Re: [PATCH 4/9] USB: Convert from tasklet to BH workqueue

2024-03-28 Thread Allen
> >
> > This patch converts drivers/infiniband/* from tasklet to BH workqueue.
> >
> > Based on the work done by Tejun Heo 
> > Branch: https://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git for-6.10
> >
> > Signed-off-by: Allen Pais 
> > ---
>
> > diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> > index c0e005670d67..88d8e1c366cd 100644
> > --- a/drivers/usb/core/hcd.c
> > +++ b/drivers/usb/core/hcd.c
>
> > @@ -1662,10 +1663,9 @@ static void __usb_hcd_giveback_urb(struct urb *urb)
> >   usb_put_urb(urb);
> >  }
> >
> > -static void usb_giveback_urb_bh(struct work_struct *work)
> > +static void usb_giveback_urb_bh(struct work_struct *t)
> >  {
> > - struct giveback_urb_bh *bh =
> > - container_of(work, struct giveback_urb_bh, bh);
> > + struct giveback_urb_bh *bh = from_work(bh, t, bh);
> >   struct list_head local_list;
> >
> >   spin_lock_irq(>lock);
>
> Is there any reason for this apparently pointless change of a local
> variable's name?

 No, it was done just to keep things consistent across the kernel.
I can revert it back to *work if you'd prefer.

Thanks.

>
> Alan Stern
>


-- 
   - Allen


Re: [PATCH 6/9] ipmi: Convert from tasklet to BH workqueue

2024-03-28 Thread Allen
On Wed, Mar 27, 2024 at 11:05 AM Corey Minyard  wrote:
>
> On Wed, Mar 27, 2024 at 04:03:11PM +, Allen Pais wrote:
> > The only generic interface to execute asynchronously in the BH context is
> > tasklet; however, it's marked deprecated and has some design flaws. To
> > replace tasklets, BH workqueue support was recently added. A BH workqueue
> > behaves similarly to regular workqueues except that the queued work items
> > are executed in the BH context.
> >
> > This patch converts drivers/infiniband/* from tasklet to BH workqueue.
>
> I think you mean drivers/char/ipmi/* here.

 My apologies, my scripts messed up the commit messages for this series.
Will have it fixed in v2.

>
> I believe that work queues items are execute single-threaded for a work
> queue, so this should be good.  I need to test this, though.  It may be
> that an IPMI device can have its own work queue; it may not be important
> to run it in bh context.

  Fair point. Could you please let me know once you have had a chance to test
these changes. Meanwhile, I will work on RFC wherein IPMI will have its own
workqueue.

 Thanks for taking time out to review.

- Allen

>
> -corey
>
> >
> > Based on the work done by Tejun Heo 
> > Branch: https://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git for-6.10
> >
> > Signed-off-by: Allen Pais 
> > ---
> >  drivers/char/ipmi/ipmi_msghandler.c | 30 ++---
> >  1 file changed, 15 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/char/ipmi/ipmi_msghandler.c 
> > b/drivers/char/ipmi/ipmi_msghandler.c
> > index b0eedc4595b3..fce2a2dbdc82 100644
> > --- a/drivers/char/ipmi/ipmi_msghandler.c
> > +++ b/drivers/char/ipmi/ipmi_msghandler.c
> > @@ -36,12 +36,13 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >
> >  #define IPMI_DRIVER_VERSION "39.2"
> >
> >  static struct ipmi_recv_msg *ipmi_alloc_recv_msg(void);
> >  static int ipmi_init_msghandler(void);
> > -static void smi_recv_tasklet(struct tasklet_struct *t);
> > +static void smi_recv_work(struct work_struct *t);
> >  static void handle_new_recv_msgs(struct ipmi_smi *intf);
> >  static void need_waiter(struct ipmi_smi *intf);
> >  static int handle_one_recv_msg(struct ipmi_smi *intf,
> > @@ -498,13 +499,13 @@ struct ipmi_smi {
> >   /*
> >* Messages queued for delivery.  If delivery fails (out of memory
> >* for instance), They will stay in here to be processed later in a
> > -  * periodic timer interrupt.  The tasklet is for handling received
> > +  * periodic timer interrupt.  The work is for handling received
> >* messages directly from the handler.
> >*/
> >   spinlock_t   waiting_rcv_msgs_lock;
> >   struct list_head waiting_rcv_msgs;
> >   atomic_t watchdog_pretimeouts_to_deliver;
> > - struct tasklet_struct recv_tasklet;
> > + struct work_struct recv_work;
> >
> >   spinlock_t xmit_msgs_lock;
> >   struct list_head   xmit_msgs;
> > @@ -704,7 +705,7 @@ static void clean_up_interface_data(struct ipmi_smi 
> > *intf)
> >   struct cmd_rcvr  *rcvr, *rcvr2;
> >   struct list_head list;
> >
> > - tasklet_kill(>recv_tasklet);
> > + cancel_work_sync(>recv_work);
> >
> >   free_smi_msg_list(>waiting_rcv_msgs);
> >   free_recv_msg_list(>waiting_events);
> > @@ -1319,7 +1320,7 @@ static void free_user(struct kref *ref)
> >  {
> >   struct ipmi_user *user = container_of(ref, struct ipmi_user, 
> > refcount);
> >
> > - /* SRCU cleanup must happen in task context. */
> > + /* SRCU cleanup must happen in work context. */
> >   queue_work(remove_work_wq, >remove_work);
> >  }
> >
> > @@ -3605,8 +3606,7 @@ int ipmi_add_smi(struct module *owner,
> >   intf->curr_seq = 0;
> >   spin_lock_init(>waiting_rcv_msgs_lock);
> >   INIT_LIST_HEAD(>waiting_rcv_msgs);
> > - tasklet_setup(>recv_tasklet,
> > -  smi_recv_tasklet);
> > + INIT_WORK(>recv_work, smi_recv_work);
> >   atomic_set(>watchdog_pretimeouts_to_deliver, 0);
> >   spin_lock_init(>xmit_msgs_lock);
> >   INIT_LIST_HEAD(>xmit_msgs);
> > @@ -4779,7 +4779,7 @@ static void handle_new_recv_msgs(struct ipmi_smi 
> > *intf)
> >* To preserve message order, quit if we
> >* can't handle a message.  Add the message
> >* back at the head, this is safe because this
> > -  * tasklet is the only thing that pulls the
> > +  * work is the only thing that pulls the
> >* messages.
> >*/
> >   list_add(_msg->link, >waiting_rcv_msgs);
> > @@ -4812,10 +4812,10 @@ static void handle_new_recv_msgs(struct ipmi_smi 
> > *intf)
> >   }
> >  }
> >
> > -static void smi_recv_tasklet(struct tasklet_struct *t)
> > +static void smi_recv_work(struct work_struct *t)
> >  {
> >   unsigned long flags = 0; /* keep 

Re: [PATCH 2/9] dma: Convert from tasklet to BH workqueue

2024-03-28 Thread Allen
>
> Subsytem is dmaengine, can you rename this to dmaengine: ...

 My apologies, will have it fixed in v2.

>
> On 27-03-24, 16:03, Allen Pais wrote:
> > The only generic interface to execute asynchronously in the BH context is
> > tasklet; however, it's marked deprecated and has some design flaws. To
> > replace tasklets, BH workqueue support was recently added. A BH workqueue
> > behaves similarly to regular workqueues except that the queued work items
> > are executed in the BH context.
>
> Thanks for conversion, am happy with BH alternative as it helps in
> dmaengine where we need shortest possible time between tasklet and
> interrupt handling to maximize dma performance
>
> >
> > This patch converts drivers/dma/* from tasklet to BH workqueue.
>
> >
> > Based on the work done by Tejun Heo 
> > Branch: git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git for-6.10
> >
> > Signed-off-by: Allen Pais 
> > ---
> >  drivers/dma/altera-msgdma.c   | 15 
> >  drivers/dma/apple-admac.c | 15 
> >  drivers/dma/at_hdmac.c|  2 +-
> >  drivers/dma/at_xdmac.c| 15 
> >  drivers/dma/bcm2835-dma.c |  2 +-
> >  drivers/dma/dma-axi-dmac.c|  2 +-
> >  drivers/dma/dma-jz4780.c  |  2 +-
> >  .../dma/dw-axi-dmac/dw-axi-dmac-platform.c|  2 +-
> >  drivers/dma/dw-edma/dw-edma-core.c|  2 +-
> >  drivers/dma/dw/core.c | 13 +++
> >  drivers/dma/dw/regs.h |  3 +-
> >  drivers/dma/ep93xx_dma.c  | 15 
> >  drivers/dma/fsl-edma-common.c |  2 +-
> >  drivers/dma/fsl-qdma.c|  2 +-
> >  drivers/dma/fsl_raid.c| 11 +++---
> >  drivers/dma/fsl_raid.h|  2 +-
> >  drivers/dma/fsldma.c  | 15 
> >  drivers/dma/fsldma.h  |  3 +-
> >  drivers/dma/hisi_dma.c|  2 +-
> >  drivers/dma/hsu/hsu.c |  2 +-
> >  drivers/dma/idma64.c  |  4 +--
> >  drivers/dma/img-mdc-dma.c |  2 +-
> >  drivers/dma/imx-dma.c | 27 +++---
> >  drivers/dma/imx-sdma.c|  6 ++--
> >  drivers/dma/ioat/dma.c| 17 -
> >  drivers/dma/ioat/dma.h|  5 +--
> >  drivers/dma/ioat/init.c   |  2 +-
> >  drivers/dma/k3dma.c   | 19 +-
> >  drivers/dma/mediatek/mtk-cqdma.c  | 35 ++-
> >  drivers/dma/mediatek/mtk-hsdma.c  |  2 +-
> >  drivers/dma/mediatek/mtk-uart-apdma.c |  4 +--
> >  drivers/dma/mmp_pdma.c| 13 +++
> >  drivers/dma/mmp_tdma.c| 11 +++---
> >  drivers/dma/mpc512x_dma.c | 17 -
> >  drivers/dma/mv_xor.c  | 13 +++
> >  drivers/dma/mv_xor.h  |  5 +--
> >  drivers/dma/mv_xor_v2.c   | 23 ++--
> >  drivers/dma/mxs-dma.c | 13 +++
> >  drivers/dma/nbpfaxi.c | 15 
> >  drivers/dma/owl-dma.c |  2 +-
> >  drivers/dma/pch_dma.c | 17 -
> >  drivers/dma/pl330.c   | 31 
> >  drivers/dma/plx_dma.c | 13 +++
> >  drivers/dma/ppc4xx/adma.c | 17 -
> >  drivers/dma/ppc4xx/adma.h |  5 +--
> >  drivers/dma/pxa_dma.c |  2 +-
> >  drivers/dma/qcom/bam_dma.c| 35 ++-
> >  drivers/dma/qcom/gpi.c| 18 +-
> >  drivers/dma/qcom/hidma.c  | 11 +++---
> >  drivers/dma/qcom/hidma.h  |  5 +--
> >  drivers/dma/qcom/hidma_ll.c   | 11 +++---
> >  drivers/dma/qcom/qcom_adm.c   |  2 +-
> >  drivers/dma/sa11x0-dma.c  | 27 +++---
> >  drivers/dma/sf-pdma/sf-pdma.c | 23 ++--
> >  drivers/dma/sf-pdma/sf-pdma.h |  5 +--
> >  drivers/dma/sprd-dma.c|  2 +-
> >  drivers/dma/st_fdma.c |  2 +-
> >  drivers/dma/ste_dma40.c   | 17 -
> >  drivers/dma/sun6i-dma.c   | 33 -
> >  drivers/dma/tegra186-gpc-dma.c|  2 +-
> >  drivers/dma/tegra20-apb-dma.c | 19 +-
> >  drivers/dma/tegra210-adma.c   |  2 +-
> >  drivers/dma/ti/edma.c |  2 +-
> >  drivers/dma/ti/k3-udma.c  | 11 +++---
> >  drivers/dma/ti/omap-dma.c

Re: [PATCH 9/9] mmc: Convert from tasklet to BH workqueue

2024-03-28 Thread Allen
On Thu, Mar 28, 2024 at 3:16 AM Christian Loehle
 wrote:
>
> On 27/03/2024 16:03, Allen Pais wrote:
> > The only generic interface to execute asynchronously in the BH context is
> > tasklet; however, it's marked deprecated and has some design flaws. To
> > replace tasklets, BH workqueue support was recently added. A BH workqueue
> > behaves similarly to regular workqueues except that the queued work items
> > are executed in the BH context.
> >
> > This patch converts drivers/infiniband/* from tasklet to BH workqueue.
> s/infiniband/mmc

Will fix it in v2.
> >
> > Based on the work done by Tejun Heo 
> > Branch: https://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git for-6.10
> >
> > Signed-off-by: Allen Pais 
> > ---
> >  drivers/mmc/host/atmel-mci.c  | 35 -
> >  drivers/mmc/host/au1xmmc.c| 37 -
> >  drivers/mmc/host/cb710-mmc.c  | 15 ++--
> >  drivers/mmc/host/cb710-mmc.h  |  3 +-
> >  drivers/mmc/host/dw_mmc.c | 25 ---
> >  drivers/mmc/host/dw_mmc.h |  9 ++-
> For dw_mmc:
> Performance numbers look good FWIW.
> for i in $(seq 0 5); do echo performance > 
> /sys/devices/system/cpu/cpu$i/cpufreq/scaling_governor; done
> for i in $(seq 0 4); do fio --name=test --rw=randread --bs=4k --runtime=30 
> --time_based --filename=/dev/mmcblk1 --minimal --numjobs=6 --iodepth=32 
> --group_reporting | awk -F ";" '{print $8}'; sleep 30; done
> Baseline:
> 1758
> 1773
> 1619
> 1835
> 1639
> to:
> 1743
> 1643
> 1860
> 1638
> 1872
> (I'd call that equivalent).
> This is on a RK3399.
> I would prefer most of the naming to change from "work" to "workqueue" in the 
> driver
> code.
> Apart from that:
> Reviewed-by: Christian Loehle 
> Tested-by: Christian Loehle 

 Thank you very much for testing and the review. Will have your
concerns addressed in v2.

- Allen


Re: [PATCH 9/9] mmc: Convert from tasklet to BH workqueue

2024-03-28 Thread Tejun Heo
Hello,

On Thu, Mar 28, 2024 at 01:53:25PM +0100, Ulf Hansson wrote:
> At this point we have suggested to drivers to switch to use threaded
> irq handlers (and regular work queues if needed too). That said,
> what's the benefit of using the BH work queue?

BH workqueues should behave about the same as tasklets which have more
limited interface and is subtly broken in an expensive-to-fix way (around
freeing in-flight work item), so the plan is to replace tasklets with BH
workqueues and remove tasklets from the kernel.

The [dis]advantages of BH workqueues over threaded IRQs or regular threaded
workqueues are the same as when you compare them to tasklets. No thread
switching overhead, so latencies will be a bit tighter. Wheteher that
actually matters really depends on the use case. Here, the biggest advantage
is that it's mostly interchangeable with tasklets and can thus be swapped
easily.

Thanks.

-- 
tejun


Re: [PATCH v4 06/13] mm/gup: Drop folio_fast_pin_allowed() in hugepd processing

2024-03-28 Thread Andrew Morton
On Thu, 28 Mar 2024 11:10:59 +0100 David Hildenbrand  wrote:

> @Andrew, you properly adjusted the code to remove the 
> gup_fast_folio_allowed() call instead of the folio_fast_pin_allowed() 
> call, but
> 
> (1) the commit subject
> (2) comment for gup_huge_pd()
> 
> Still mention folio_fast_pin_allowed().
> 
> The patch "mm/gup: handle hugepd for follow_page()" then moves that 
> (outdated) comment.

OK, thanks, I fixed all that up.


Re: [PATCH v7 6/6] docs: trusted-encrypted: add DCP as new trust source

2024-03-28 Thread Jarkko Sakkinen
On Thu Mar 28, 2024 at 8:47 PM EET, Jarkko Sakkinen wrote:
> On Thu Mar 28, 2024 at 10:05 AM EET, David Gstir wrote:
> > Jarkko,
> >
> > > On 27.03.2024, at 16:40, Jarkko Sakkinen  wrote:
> > > 
> > > On Wed Mar 27, 2024 at 10:24 AM EET, David Gstir wrote:
> > >> Update the documentation for trusted and encrypted KEYS with DCP as new
> > >> trust source:
> > >> 
> > >> - Describe security properties of DCP trust source
> > >> - Describe key usage
> > >> - Document blob format
> > >> 
> > >> Co-developed-by: Richard Weinberger 
> > >> Signed-off-by: Richard Weinberger 
> > >> Co-developed-by: David Oberhollenzer 
> > >> Signed-off-by: David Oberhollenzer 
> > >> Signed-off-by: David Gstir 
> > >> ---
> > >> .../security/keys/trusted-encrypted.rst   | 85 +++
> > >> 1 file changed, 85 insertions(+)
> > >> 
> > >> diff --git a/Documentation/security/keys/trusted-encrypted.rst 
> > >> b/Documentation/security/keys/trusted-encrypted.rst
> > >> index e989b9802f92..81fb3540bb20 100644
> > >> --- a/Documentation/security/keys/trusted-encrypted.rst
> > >> +++ b/Documentation/security/keys/trusted-encrypted.rst
> > >> @@ -42,6 +42,14 @@ safe.
> > >>  randomly generated and fused into each SoC at manufacturing 
> > >> time.
> > >>  Otherwise, a common fixed test key is used instead.
> > >> 
> > >> + (4) DCP (Data Co-Processor: crypto accelerator of various i.MX 
> > >> SoCs)
> > >> +
> > >> + Rooted to a one-time programmable key (OTP) that is generally 
> > >> burnt
> > >> + in the on-chip fuses and is accessible to the DCP encryption 
> > >> engine only.
> > >> + DCP provides two keys that can be used as root of trust: the 
> > >> OTP key
> > >> + and the UNIQUE key. Default is to use the UNIQUE key, but 
> > >> selecting
> > >> + the OTP key can be done via a module parameter 
> > >> (dcp_use_otp_key).
> > >> +
> > >>   *  Execution isolation
> > >> 
> > >>  (1) TPM
> > >> @@ -57,6 +65,12 @@ safe.
> > >> 
> > >>  Fixed set of operations running in isolated execution 
> > >> environment.
> > >> 
> > >> + (4) DCP
> > >> +
> > >> + Fixed set of cryptographic operations running in isolated 
> > >> execution
> > >> + environment. Only basic blob key encryption is executed there.
> > >> + The actual key sealing/unsealing is done on main 
> > >> processor/kernel space.
> > >> +
> > >>   * Optional binding to platform integrity state
> > >> 
> > >>  (1) TPM
> > >> @@ -79,6 +93,11 @@ safe.
> > >>  Relies on the High Assurance Boot (HAB) mechanism of NXP SoCs
> > >>  for platform integrity.
> > >> 
> > >> + (4) DCP
> > >> +
> > >> + Relies on Secure/Trusted boot process (called HAB by vendor) 
> > >> for
> > >> + platform integrity.
> > >> +
> > >>   *  Interfaces and APIs
> > >> 
> > >>  (1) TPM
> > >> @@ -94,6 +113,11 @@ safe.
> > >> 
> > >>  Interface is specific to silicon vendor.
> > >> 
> > >> + (4) DCP
> > >> +
> > >> + Vendor-specific API that is implemented as part of the DCP 
> > >> crypto driver in
> > >> + ``drivers/crypto/mxs-dcp.c``.
> > >> +
> > >>   *  Threat model
> > >> 
> > >>  The strength and appropriateness of a particular trust source for a 
> > >> given
> > >> @@ -129,6 +153,13 @@ selected trust source:
> > >>  CAAM HWRNG, enable CRYPTO_DEV_FSL_CAAM_RNG_API and ensure the device
> > >>  is probed.
> > >> 
> > >> +  *  DCP (Data Co-Processor: crypto accelerator of various i.MX SoCs)
> > >> +
> > >> + The DCP hardware device itself does not provide a dedicated RNG 
> > >> interface,
> > >> + so the kernel default RNG is used. SoCs with DCP like the i.MX6ULL 
> > >> do have
> > >> + a dedicated hardware RNG that is independent from DCP which can be 
> > >> enabled
> > >> + to back the kernel RNG.
> > >> +
> > >> Users may override this by specifying ``trusted.rng=kernel`` on the 
> > >> kernel
> > >> command-line to override the used RNG with the kernel's random number 
> > >> pool.
> > >> 
> > >> @@ -231,6 +262,19 @@ Usage::
> > >> CAAM-specific format.  The key length for new keys is always in bytes.
> > >> Trusted Keys can be 32 - 128 bytes (256 - 1024 bits).
> > >> 
> > >> +Trusted Keys usage: DCP
> > >> +---
> > >> +
> > >> +Usage::
> > >> +
> > >> +keyctl add trusted name "new keylen" ring
> > >> +keyctl add trusted name "load hex_blob" ring
> > >> +keyctl print keyid
> > >> +
> > >> +"keyctl print" returns an ASCII hex copy of the sealed key, which is in 
> > >> format
> > >> +specific to this DCP key-blob implementation.  The key length for new 
> > >> keys is
> > >> +always in bytes. Trusted Keys can be 32 - 128 bytes (256 - 1024 bits).
> > >> +
> > >> Encrypted Keys usage
> > >> 
> > >> 
> > >> @@ -426,3 +470,44 @@ string length.
> > >> privkey is the binary representation of TPM2B_PUBLIC excluding the
> > >> initial TPM2B 

Re: [PATCH v7 6/6] docs: trusted-encrypted: add DCP as new trust source

2024-03-28 Thread Jarkko Sakkinen
On Thu Mar 28, 2024 at 10:05 AM EET, David Gstir wrote:
> Jarkko,
>
> > On 27.03.2024, at 16:40, Jarkko Sakkinen  wrote:
> > 
> > On Wed Mar 27, 2024 at 10:24 AM EET, David Gstir wrote:
> >> Update the documentation for trusted and encrypted KEYS with DCP as new
> >> trust source:
> >> 
> >> - Describe security properties of DCP trust source
> >> - Describe key usage
> >> - Document blob format
> >> 
> >> Co-developed-by: Richard Weinberger 
> >> Signed-off-by: Richard Weinberger 
> >> Co-developed-by: David Oberhollenzer 
> >> Signed-off-by: David Oberhollenzer 
> >> Signed-off-by: David Gstir 
> >> ---
> >> .../security/keys/trusted-encrypted.rst   | 85 +++
> >> 1 file changed, 85 insertions(+)
> >> 
> >> diff --git a/Documentation/security/keys/trusted-encrypted.rst 
> >> b/Documentation/security/keys/trusted-encrypted.rst
> >> index e989b9802f92..81fb3540bb20 100644
> >> --- a/Documentation/security/keys/trusted-encrypted.rst
> >> +++ b/Documentation/security/keys/trusted-encrypted.rst
> >> @@ -42,6 +42,14 @@ safe.
> >>  randomly generated and fused into each SoC at manufacturing time.
> >>  Otherwise, a common fixed test key is used instead.
> >> 
> >> + (4) DCP (Data Co-Processor: crypto accelerator of various i.MX SoCs)
> >> +
> >> + Rooted to a one-time programmable key (OTP) that is generally 
> >> burnt
> >> + in the on-chip fuses and is accessible to the DCP encryption 
> >> engine only.
> >> + DCP provides two keys that can be used as root of trust: the OTP 
> >> key
> >> + and the UNIQUE key. Default is to use the UNIQUE key, but 
> >> selecting
> >> + the OTP key can be done via a module parameter (dcp_use_otp_key).
> >> +
> >>   *  Execution isolation
> >> 
> >>  (1) TPM
> >> @@ -57,6 +65,12 @@ safe.
> >> 
> >>  Fixed set of operations running in isolated execution environment.
> >> 
> >> + (4) DCP
> >> +
> >> + Fixed set of cryptographic operations running in isolated 
> >> execution
> >> + environment. Only basic blob key encryption is executed there.
> >> + The actual key sealing/unsealing is done on main 
> >> processor/kernel space.
> >> +
> >>   * Optional binding to platform integrity state
> >> 
> >>  (1) TPM
> >> @@ -79,6 +93,11 @@ safe.
> >>  Relies on the High Assurance Boot (HAB) mechanism of NXP SoCs
> >>  for platform integrity.
> >> 
> >> + (4) DCP
> >> +
> >> + Relies on Secure/Trusted boot process (called HAB by vendor) for
> >> + platform integrity.
> >> +
> >>   *  Interfaces and APIs
> >> 
> >>  (1) TPM
> >> @@ -94,6 +113,11 @@ safe.
> >> 
> >>  Interface is specific to silicon vendor.
> >> 
> >> + (4) DCP
> >> +
> >> + Vendor-specific API that is implemented as part of the DCP 
> >> crypto driver in
> >> + ``drivers/crypto/mxs-dcp.c``.
> >> +
> >>   *  Threat model
> >> 
> >>  The strength and appropriateness of a particular trust source for a 
> >> given
> >> @@ -129,6 +153,13 @@ selected trust source:
> >>  CAAM HWRNG, enable CRYPTO_DEV_FSL_CAAM_RNG_API and ensure the device
> >>  is probed.
> >> 
> >> +  *  DCP (Data Co-Processor: crypto accelerator of various i.MX SoCs)
> >> +
> >> + The DCP hardware device itself does not provide a dedicated RNG 
> >> interface,
> >> + so the kernel default RNG is used. SoCs with DCP like the i.MX6ULL 
> >> do have
> >> + a dedicated hardware RNG that is independent from DCP which can be 
> >> enabled
> >> + to back the kernel RNG.
> >> +
> >> Users may override this by specifying ``trusted.rng=kernel`` on the kernel
> >> command-line to override the used RNG with the kernel's random number pool.
> >> 
> >> @@ -231,6 +262,19 @@ Usage::
> >> CAAM-specific format.  The key length for new keys is always in bytes.
> >> Trusted Keys can be 32 - 128 bytes (256 - 1024 bits).
> >> 
> >> +Trusted Keys usage: DCP
> >> +---
> >> +
> >> +Usage::
> >> +
> >> +keyctl add trusted name "new keylen" ring
> >> +keyctl add trusted name "load hex_blob" ring
> >> +keyctl print keyid
> >> +
> >> +"keyctl print" returns an ASCII hex copy of the sealed key, which is in 
> >> format
> >> +specific to this DCP key-blob implementation.  The key length for new 
> >> keys is
> >> +always in bytes. Trusted Keys can be 32 - 128 bytes (256 - 1024 bits).
> >> +
> >> Encrypted Keys usage
> >> 
> >> 
> >> @@ -426,3 +470,44 @@ string length.
> >> privkey is the binary representation of TPM2B_PUBLIC excluding the
> >> initial TPM2B header which can be reconstructed from the ASN.1 octed
> >> string length.
> >> +
> >> +DCP Blob Format
> >> +---
> >> +
> >> +The Data Co-Processor (DCP) provides hardware-bound AES keys using its
> >> +AES encryption engine only. It does not provide direct key 
> >> sealing/unsealing.
> >> +To make DCP hardware encryption keys usable as trust source, we define

Re: Appropriate liburcu cache line size for Power

2024-03-28 Thread Mathieu Desnoyers

On 2024-03-25 16:34, Nathan Lynch wrote:

Mathieu Desnoyers  writes:

In the powerpc architecture support within the liburcu project [1]
we have a cache line size defined as 256 bytes with the following
comment:

/* Include size of POWER5+ L3 cache lines: 256 bytes */
#define CAA_CACHE_LINE_SIZE 256

I recently received a pull request on github [2] asking to
change this to 128 bytes. All the material provided supports
that the cache line sizes on powerpc are 128 bytes or less (even
L3 on POWER7, POWER8, and POWER9) [3].

I wonder where the 256 bytes L3 cache line size for POWER5+
we have in liburcu comes from, and I wonder if it's the right choice
for a cache line size on all powerpc, considering that the Linux
kernel cache line size appear to use 128 bytes on recent Power
architectures. I recall some benchmark experiments Paul and I did
on a 64-core 1.9GHz POWER5+ machine that benefited from a 256 bytes
cache line size, and I suppose this is why we came up with this
value, but I don't have the detailed specs of that machine.

Any feedback on this matter would be appreciated.


For what it's worth, I found a copy of an IBM Journal of Research &
Development article confirming that POWER5's L3 had a 256-byte line
size:

   Each slice [of the L3] is 12-way set-associative, with 4,096
   congruence classes of 256-byte lines managed as two 128-byte sectors
   to match the L2 line size.

https://www.eecg.utoronto.ca/~moshovos/ACA08/readings/power5.pdf

I don't know of any reason to prefer 256 over 128 for current Power
processors though.


Thanks for the pointer. I will add a reference to it in the liburcu
source code to explain the cache line size choice.

Mathieu


--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com



Re: [PATCH RFC 0/3] mm/gup: consistently call it GUP-fast

2024-03-28 Thread Vineet Gupta


On 3/28/24 00:15, Mike Rapoport wrote:
> On Thu, Mar 28, 2024 at 07:09:13AM +0100, Arnd Bergmann wrote:
>> On Thu, Mar 28, 2024, at 06:51, Vineet Gupta wrote:
>>> On 3/27/24 09:22, Arnd Bergmann wrote:
 On Wed, Mar 27, 2024, at 16:39, David Hildenbrand wrote:
> On 27.03.24 16:21, Peter Xu wrote:
>> On Wed, Mar 27, 2024 at 02:05:35PM +0100, David Hildenbrand wrote:
>>
>> I'm not sure what config you tried there; as I am doing some build tests
>> recently, I found turning off CONFIG_SAMPLES + CONFIG_GCC_PLUGINS could
>> avoid a lot of issues, I think it's due to libc missing.  But maybe not 
>> the
>> case there.
> CCin Arnd; I use some of his compiler chains, others from Fedora 
> directly. For
> example for alpha and arc, the Fedora gcc is "13.2.1".
> But there is other stuff like (arc):
>
> ./arch/arc/include/asm/mmu-arcv2.h: In function 'mmu_setup_asid':
> ./arch/arc/include/asm/mmu-arcv2.h:82:9: error: implicit declaration of 
> function 'write_aux_reg' [-Werro
> r=implicit-function-declaration]
> 82 | write_aux_reg(ARC_REG_PID, asid | MMU_ENABLE);
>| ^
 Seems to be missing an #include of soc/arc/aux.h, but I can't
 tell when this first broke without bisecting.
>>> Weird I don't see this one but I only have gcc 12 handy ATM.
>>>
>>>     gcc version 12.2.1 20230306 (ARC HS GNU/Linux glibc toolchain -
>>> build 1360)
>>>
>>> I even tried W=1 (which according to scripts/Makefile.extrawarn) should
>>> include -Werror=implicit-function-declaration but don't see this still.
>>>
>>> Tomorrow I'll try building a gcc 13.2.1 for ARC.
>> David reported them with the toolchains I built at
>> https://mirrors.edge.kernel.org/pub/tools/crosstool/
>> I'm fairly sure the problem is specific to the .config
>> and tree, not the toolchain though.
> This happens with defconfig and both gcc 12.2.0 and gcc 13.2.0 from your
> crosstools. I also see these on the current Linus' tree:
>
> arc/kernel/ptrace.c:342:16: warning: no previous prototype for 
> 'syscall_trace_enter' [-Wmissing-prototypes]
> arch/arc/kernel/kprobes.c:193:15: warning: no previous prototype for 
> 'arc_kprobe_handler' [-Wmissing-prototypes]

Yep these two I could trigger and fix posted [1]

> This fixed the warning about write_aux_reg for me, probably Vineet would
> want this include somewhere else...
>
> diff --git a/arch/arc/include/asm/mmu-arcv2.h 
> b/arch/arc/include/asm/mmu-arcv2.h
> index ed9036d4ede3..0fca342d7b79 100644
> --- a/arch/arc/include/asm/mmu-arcv2.h
> +++ b/arch/arc/include/asm/mmu-arcv2.h
> @@ -69,6 +69,8 @@
>  
>  #ifndef __ASSEMBLY__
>  
> +#include 
> +
>  struct mm_struct;
>  extern int pae40_exist_but_not_enab(void);

Thx Mike. Indeed the fix is trivial but on tip of tree I still can't
trigger the warning to even test anything. I'm at following with my
other fixes.

    2024-03-27 962490525cff Merge tag 'probes-fixes-v6.9-rc1' of
git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace  

I tried defconfig build as well as the exact config from Linaro report
[2], and/or various toolchains: from snps github, Arnd's crosstool
toolchain.
Granted all of these are linux toolchains - I vaguely remember at some
time, baremetal elf32 toolchain behaved differently due to different
defaults etc.
I have a feeling this was something transient which got fixed up due to
order of header includes etc.

Anyone in the followup email David only reported 2 warnings which have
been tended to as mentioned above - will be sent to Linus soon.

[1]
http://lists.infradead.org/pipermail/linux-snps-arc/2024-March/007916.html
[2]
https://storage.tuxsuite.com/public/linaro/lkft/builds/2eA2VSZdDsL0DMBBhjoauN9IVoK/

Thx,
-Vineet


Re: [kvm-unit-tests PATCH v7 07/35] common: add memory dirtying vs migration test

2024-03-28 Thread Thomas Huth

On 19/03/2024 08.58, Nicholas Piggin wrote:

This test stores to a bunch of pages and verifies previous stores,
while being continually migrated. Default runtime is 5 seconds.

Add this test to ppc64 and s390x builds. This can fail due to a QEMU
TCG physical memory dirty bitmap bug, so it is not enabled in unittests
for TCG yet.

The selftest-migration test time is reduced significantly because
this test

Signed-off-by: Nicholas Piggin 
---
  common/memory-verify.c  | 67 +
  common/selftest-migration.c |  8 ++---
  powerpc/Makefile.common |  1 +
  powerpc/memory-verify.c |  1 +
  powerpc/unittests.cfg   |  7 
  s390x/Makefile  |  1 +
  s390x/memory-verify.c   |  1 +
  s390x/unittests.cfg |  6 
  8 files changed, 88 insertions(+), 4 deletions(-)
  create mode 100644 common/memory-verify.c
  create mode 12 powerpc/memory-verify.c
  create mode 12 s390x/memory-verify.c

diff --git a/common/memory-verify.c b/common/memory-verify.c
new file mode 100644
index 0..e78fb4338
--- /dev/null
+++ b/common/memory-verify.c
@@ -0,0 +1,67 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Simple memory verification test, used to exercise dirty memory migration.
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define NR_PAGES 32
+
+static unsigned time_sec = 5;
+
+static void do_getopts(int argc, char **argv)
+{
+   int i;
+
+   for (i = 0; i < argc; ++i) {
+   if (strcmp(argv[i], "-t") == 0) {
+   i++;
+   if (i == argc)
+   break;
+   time_sec = atol(argv[i]);
+   }
+   }
+
+   printf("running for %d secs\n", time_sec);
+}
+
+int main(int argc, char **argv)
+{
+   void *mem = malloc(NR_PAGES*PAGE_SIZE);


Use alloc_pages(5) instead ? Or add at least some white spaces around "*".

Apart from that this patch looks sane to me, so with that line fixed:
Reviewed-by: Thomas Huth 



Re: [PATCH v2 2/3] arch: Remove struct fb_info from video helpers

2024-03-28 Thread Helge Deller

On 3/28/24 14:33, Thomas Zimmermann wrote:

Am 28.03.24 um 12:04 schrieb Helge Deller:

On 3/27/24 21:41, Thomas Zimmermann wrote:

The per-architecture video helpers do not depend on struct fb_info
or anything else from fbdev. Remove it from the interface and replace
fb_is_primary_device() with video_is_primary_device(). The new helper


Since you rename this function, wouldn't something similar to

device_is_primary_display()
or
device_is_primary_console()
or
is_primary_graphics_device()
or
is_primary_display_device()

be a better name?


The video_ prefix is there to signal that the code is part of the video 
subsystem.

But there's too much code that tried to figure out a default video
device. So I actually have different plans for this function. I'd
like to replace it with a helper that returns the default device
instead of just testing for it. Sample code for x86 is already in
vgaarb.c. [1] The function's name would then be
video_default_device() and return the appropriate struct device*.
video_is_primary device() will be removed. This rename here is the
easiest step towards the new helper. Ok?

Sounds ok.

Helge


Re: [PATCH v2 4/6] mm/mm_init.c: remove meaningless calculation of zone->managed_pages in free_area_init_core()

2024-03-28 Thread Baoquan He
On 03/28/24 at 11:53am, Mike Rapoport wrote:
> On Thu, Mar 28, 2024 at 04:32:38PM +0800, Baoquan He wrote:
> > On 03/25/24 at 10:56pm, Baoquan He wrote:
> > >  
> > >   /*
> > > -  * Set an approximate value for lowmem here, it will be adjusted
> > > -  * when the bootmem allocator frees pages into the buddy system.
> > > -  * And all highmem pages will be managed by the buddy system.
> > > +  * Initialize zone->managed_pages as 0 , it will be reset
> > > +  * when memblock allocator frees pages into buddy system.
> > >*/
> > > - zone_init_internals(zone, j, nid, freesize);
> > > + zone_init_internals(zone, j, nid, 0);
> > 
> > Here, we should initialize zone->managed_pages as zone->present_pages
> > because later page_group_by_mobility_disabled need be set according to
> > zone->managed_pages. Otherwise page_group_by_mobility_disabled will be
> > set to 1 always. I will sent out v3.
> 
> With zone->managed_pages set to zone->present_pages we won't account for
> the reserved memory for initialization of page_group_by_mobility_disabled.

The old zone->managed_pages didn't account for the reserved pages
either. It's calculated by (zone->present_pages - memmap_pages). memmap
pages only is only a very small portion, e.g on x86_64, 4K page size,
assuming size of struct page is 64, then it's 1/64 of system memory.
On arm64, 64K page size, it's 1/1024 of system memory.

And about the setting of page_group_by_mobility_disabled, the compared
value pageblock_nr_pages * MIGRATE_TYPES which is very small. On x86_64,
it's 4M*6=24M; on arm64 with 64K size and 128M*6=768M which should be
the biggest among ARCH-es. 

if (vm_total_pages < (pageblock_nr_pages * MIGRATE_TYPES)) 
page_group_by_mobility_disabled = 1;
else
page_group_by_mobility_disabled = 0;

So page_group_by_mobility_disabled could be set to 1 only on system with
very little memory which is very rarely seen. And setting
zone->managed_pages as zone->present_pages is very close to its old
value: (zone->present_pages - memmap_pages). Here we don't need be very
accurate, just a rough value.

> 
> As watermarks are still not initialized at the time build_all_zonelists()
> is called, we may use nr_all_pages - nr_kernel_pages instead of
> nr_free_zone_pages(), IMO.

nr_all_pages should be fine if we take this way. nr_kernel_pages is a
misleading name, it's all low memory pages excluding kernel reserved
apges. nr_all_pages is all memory pages including highmema and exluding
kernel reserved pages.

Both is fine to me. The first one is easier, simply setting
zone->managed_pages as zone->present_pages. The 2nd way is a little more
accurate.

>  
> > From a17b0921b4bd00596330f61ee9ea4b82386a9fed Mon Sep 17 00:00:00 2001
> > From: Baoquan He 
> > Date: Thu, 28 Mar 2024 16:20:15 +0800
> > Subject: [PATCH] mm/mm_init.c: set zone's ->managed_pages as ->present_pages
> >  for now
> > Content-type: text/plain
> > 
> > Because page_group_by_mobility_disabled need be set according to zone's
> > managed_pages later.
> > 
> > Signed-off-by: Baoquan He 
> > ---
> >  mm/mm_init.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/mm/mm_init.c b/mm/mm_init.c
> > index cc24e7958c0c..dd875f943cbb 100644
> > --- a/mm/mm_init.c
> > +++ b/mm/mm_init.c
> > @@ -1561,7 +1561,7 @@ static void __init free_area_init_core(struct 
> > pglist_data *pgdat)
> >  * Initialize zone->managed_pages as 0 , it will be reset
> >  * when memblock allocator frees pages into buddy system.
> >  */
> > -   zone_init_internals(zone, j, nid, 0);
> > +   zone_init_internals(zone, j, nid, zone->present_pages);
> >  
> > if (!size)
> > continue;
> > -- 
> > 2.41.0
> > 
> > 
> > >  
> > >   if (!size)
> > >   continue;
> > > @@ -1915,6 +1878,7 @@ void __init free_area_init(unsigned long 
> > > *max_zone_pfn)
> > >   check_for_memory(pgdat);
> > >   }
> > >  
> > > + calc_nr_kernel_pages();
> > >   memmap_init();
> > >  
> > >   /* disable hash distribution for systems with a single node */
> > > -- 
> > > 2.41.0
> > > 
> > 
> 
> -- 
> Sincerely yours,
> Mike.
> 



Re: [PATCH 9/9] mmc: Convert from tasklet to BH workqueue

2024-03-28 Thread Linus Walleij
On Thu, Mar 28, 2024 at 1:54 PM Ulf Hansson  wrote:

> At this point we have suggested to drivers to switch to use threaded
> irq handlers (and regular work queues if needed too). That said,
> what's the benefit of using the BH work queue?

Context:
https://lwn.net/Articles/960041/
"Tasklets, in particular, remain because they offer lower latency than
workqueues which, since they must go through the CPU scheduler,
can take longer to execute a deferred-work item."

The BH WQ is controlled by a software IRQ and quicker than an
ordinary work item.

I don't know if this little latency could actually affect any MMC
device, I doubt it.

The other benefit IIUC is that it is easy to mechanically rewrite tasklets
to BH workqueues and be sure that it is as fast as the tasklet, if you want
to switch to threaded IRQ handlers or proper work, you need to write a
lot of elaborate code and test it (preferably on real hardware).

Yours,
Linus Walleij


Re: [PATCH 9/9] mmc: Convert from tasklet to BH workqueue

2024-03-28 Thread Ulf Hansson
On Wed, 27 Mar 2024 at 17:03, Allen Pais  wrote:
>
> The only generic interface to execute asynchronously in the BH context is
> tasklet; however, it's marked deprecated and has some design flaws. To
> replace tasklets, BH workqueue support was recently added. A BH workqueue
> behaves similarly to regular workqueues except that the queued work items
> are executed in the BH context.
>
> This patch converts drivers/infiniband/* from tasklet to BH workqueue.
>
> Based on the work done by Tejun Heo 
> Branch: https://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git for-6.10
>
> Signed-off-by: Allen Pais 

Overall, this makes sense to me. However, just to make things clear,
an mmc host driver shouldn't really need the tasklet. As I understand
it, the few remaining users are there for legacy reasons.

At this point we have suggested to drivers to switch to use threaded
irq handlers (and regular work queues if needed too). That said,
what's the benefit of using the BH work queue?

Kind regards
Uffe

> ---
>  drivers/mmc/host/atmel-mci.c  | 35 -
>  drivers/mmc/host/au1xmmc.c| 37 -
>  drivers/mmc/host/cb710-mmc.c  | 15 ++--
>  drivers/mmc/host/cb710-mmc.h  |  3 +-
>  drivers/mmc/host/dw_mmc.c | 25 ---
>  drivers/mmc/host/dw_mmc.h |  9 ++-
>  drivers/mmc/host/omap.c   | 17 +++--
>  drivers/mmc/host/renesas_sdhi.h   |  3 +-
>  drivers/mmc/host/renesas_sdhi_internal_dmac.c | 24 +++---
>  drivers/mmc/host/renesas_sdhi_sys_dmac.c  |  9 +--
>  drivers/mmc/host/sdhci-bcm-kona.c |  2 +-
>  drivers/mmc/host/tifm_sd.c| 15 ++--
>  drivers/mmc/host/tmio_mmc.h   |  3 +-
>  drivers/mmc/host/tmio_mmc_core.c  |  4 +-
>  drivers/mmc/host/uniphier-sd.c| 13 ++--
>  drivers/mmc/host/via-sdmmc.c  | 25 ---
>  drivers/mmc/host/wbsd.c   | 75 ++-
>  drivers/mmc/host/wbsd.h   | 10 +--
>  18 files changed, 167 insertions(+), 157 deletions(-)
>
> diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c
> index dba826db739a..0a92a7fd020f 100644
> --- a/drivers/mmc/host/atmel-mci.c
> +++ b/drivers/mmc/host/atmel-mci.c
> @@ -33,6 +33,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>  #include 
>  #include 
> @@ -284,12 +285,12 @@ struct atmel_mci_dma {
>   * EVENT_DATA_ERROR is pending.
>   * @stop_cmdr: Value to be loaded into CMDR when the stop command is
>   * to be sent.
> - * @tasklet: Tasklet running the request state machine.
> + * @work: Work running the request state machine.
>   * @pending_events: Bitmask of events flagged by the interrupt handler
> - * to be processed by the tasklet.
> + * to be processed by the work.
>   * @completed_events: Bitmask of events which the state machine has
>   * processed.
> - * @state: Tasklet state.
> + * @state: Work state.
>   * @queue: List of slots waiting for access to the controller.
>   * @need_clock_update: Update the clock rate before the next request.
>   * @need_reset: Reset controller before next request.
> @@ -363,7 +364,7 @@ struct atmel_mci {
> u32 data_status;
> u32 stop_cmdr;
>
> -   struct tasklet_struct   tasklet;
> +   struct work_struct  work;
> unsigned long   pending_events;
> unsigned long   completed_events;
> enum atmel_mci_statestate;
> @@ -761,7 +762,7 @@ static void atmci_timeout_timer(struct timer_list *t)
> host->need_reset = 1;
> host->state = STATE_END_REQUEST;
> smp_wmb();
> -   tasklet_schedule(>tasklet);
> +   queue_work(system_bh_wq, >work);
>  }
>
>  static inline unsigned int atmci_ns_to_clocks(struct atmel_mci *host,
> @@ -983,7 +984,7 @@ static void atmci_pdc_complete(struct atmel_mci *host)
>
> dev_dbg(>pdev->dev, "(%s) set pending xfer complete\n", 
> __func__);
> atmci_set_pending(host, EVENT_XFER_COMPLETE);
> -   tasklet_schedule(>tasklet);
> +   queue_work(system_bh_wq, >work);
>  }
>
>  static void atmci_dma_cleanup(struct atmel_mci *host)
> @@ -997,7 +998,7 @@ static void atmci_dma_cleanup(struct atmel_mci *host)
>  }
>
>  /*
> - * This function is called by the DMA driver from tasklet context.
> + * This function is called by the DMA driver from work context.
>   */
>  static void atmci_dma_complete(void *arg)
>  {
> @@ -1020,7 +1021,7 @@ static void atmci_dma_complete(void *arg)
> dev_dbg(>pdev->dev,
> "(%s) set pending xfer complete\n", __func__);
> atmci_set_pending(host, EVENT_XFER_COMPLETE);
> -   tasklet_schedule(>tasklet);
> +   queue_work(system_bh_wq, >work);
>
> /*
>  * Regardless of what the documentation 

Re: [PATCH 9/9] mmc: Convert from tasklet to BH workqueue

2024-03-28 Thread Christian Loehle
On 27/03/2024 16:03, Allen Pais wrote:
> The only generic interface to execute asynchronously in the BH context is
> tasklet; however, it's marked deprecated and has some design flaws. To
> replace tasklets, BH workqueue support was recently added. A BH workqueue
> behaves similarly to regular workqueues except that the queued work items
> are executed in the BH context.
> 
> This patch converts drivers/infiniband/* from tasklet to BH workqueue.
s/infiniband/mmc
> 
> Based on the work done by Tejun Heo 
> Branch: https://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git for-6.10
> 
> Signed-off-by: Allen Pais 
> ---
>  drivers/mmc/host/atmel-mci.c  | 35 -
>  drivers/mmc/host/au1xmmc.c| 37 -
>  drivers/mmc/host/cb710-mmc.c  | 15 ++--
>  drivers/mmc/host/cb710-mmc.h  |  3 +-
>  drivers/mmc/host/dw_mmc.c | 25 ---
>  drivers/mmc/host/dw_mmc.h |  9 ++-
For dw_mmc:
Performance numbers look good FWIW.
for i in $(seq 0 5); do echo performance > 
/sys/devices/system/cpu/cpu$i/cpufreq/scaling_governor; done
for i in $(seq 0 4); do fio --name=test --rw=randread --bs=4k --runtime=30 
--time_based --filename=/dev/mmcblk1 --minimal --numjobs=6 --iodepth=32 
--group_reporting | awk -F ";" '{print $8}'; sleep 30; done
Baseline:
1758
1773
1619
1835
1639
to:
1743
1643
1860
1638
1872
(I'd call that equivalent).
This is on a RK3399.
I would prefer most of the naming to change from "work" to "workqueue" in the 
driver
code.
Apart from that:
Reviewed-by: Christian Loehle 
Tested-by: Christian Loehle 
>  drivers/mmc/host/omap.c   | 17 +++--
>  drivers/mmc/host/renesas_sdhi.h   |  3 +-
>  drivers/mmc/host/renesas_sdhi_internal_dmac.c | 24 +++---
See inline
>  drivers/mmc/host/renesas_sdhi_sys_dmac.c  |  9 +--
>  drivers/mmc/host/sdhci-bcm-kona.c |  2 +-
>  drivers/mmc/host/tifm_sd.c| 15 ++--
>  drivers/mmc/host/tmio_mmc.h   |  3 +-
>  drivers/mmc/host/tmio_mmc_core.c  |  4 +-
>  drivers/mmc/host/uniphier-sd.c| 13 ++--
>  drivers/mmc/host/via-sdmmc.c  | 25 ---
>  drivers/mmc/host/wbsd.c   | 75 ++-
>  drivers/mmc/host/wbsd.h   | 10 +--
>  18 files changed, 167 insertions(+), 157 deletions(-)
> 
> diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c
> index dba826db739a..0a92a7fd020f 100644
> --- a/drivers/mmc/host/atmel-mci.c
> +++ b/drivers/mmc/host/atmel-mci.c
> @@ -33,6 +33,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
> @@ -284,12 +285,12 @@ struct atmel_mci_dma {
>   *   EVENT_DATA_ERROR is pending.
>   * @stop_cmdr: Value to be loaded into CMDR when the stop command is
>   *   to be sent.
> - * @tasklet: Tasklet running the request state machine.
> + * @work: Work running the request state machine.
>   * @pending_events: Bitmask of events flagged by the interrupt handler
> - *   to be processed by the tasklet.
> + *   to be processed by the work.
>   * @completed_events: Bitmask of events which the state machine has
>   *   processed.
> - * @state: Tasklet state.
> + * @state: Work state.
>   * @queue: List of slots waiting for access to the controller.
>   * @need_clock_update: Update the clock rate before the next request.
>   * @need_reset: Reset controller before next request.
> @@ -363,7 +364,7 @@ struct atmel_mci {
>   u32 data_status;
>   u32 stop_cmdr;
>  
> - struct tasklet_struct   tasklet;
> + struct work_struct  work;
>   unsigned long   pending_events;
>   unsigned long   completed_events;
>   enum atmel_mci_statestate;
> @@ -761,7 +762,7 @@ static void atmci_timeout_timer(struct timer_list *t)
>   host->need_reset = 1;
>   host->state = STATE_END_REQUEST;
>   smp_wmb();
> - tasklet_schedule(>tasklet);
> + queue_work(system_bh_wq, >work);
>  }
>  
>  static inline unsigned int atmci_ns_to_clocks(struct atmel_mci *host,
> @@ -983,7 +984,7 @@ static void atmci_pdc_complete(struct atmel_mci *host)
>  
>   dev_dbg(>pdev->dev, "(%s) set pending xfer complete\n", __func__);
>   atmci_set_pending(host, EVENT_XFER_COMPLETE);
> - tasklet_schedule(>tasklet);
> + queue_work(system_bh_wq, >work);
>  }
>  
>  static void atmci_dma_cleanup(struct atmel_mci *host)
> @@ -997,7 +998,7 @@ static void atmci_dma_cleanup(struct atmel_mci *host)
>  }
>  
>  /*
> - * This function is called by the DMA driver from tasklet context.
> + * This function is called by the DMA driver from work context.
>   */
>  static void atmci_dma_complete(void *arg)
>  {
> @@ -1020,7 +1021,7 @@ static void atmci_dma_complete(void *arg)
>   dev_dbg(>pdev->dev,
>   "(%s) set pending xfer complete\n", __func__);
>   

Re: [PATCH 2/9] dma: Convert from tasklet to BH workqueue

2024-03-28 Thread Arnd Bergmann
On Thu, Mar 28, 2024, at 06:55, Vinod Koul wrote:
> On 27-03-24, 16:03, Allen Pais wrote:
>> The only generic interface to execute asynchronously in the BH context is
>> tasklet; however, it's marked deprecated and has some design flaws. To
>> replace tasklets, BH workqueue support was recently added. A BH workqueue
>> behaves similarly to regular workqueues except that the queued work items
>> are executed in the BH context.
>
> Thanks for conversion, am happy with BH alternative as it helps in
> dmaengine where we need shortest possible time between tasklet and
> interrupt handling to maximize dma performance

I still feel that we want something different for dmaengine,
at least in the long run. As we have discussed in the past,
the tasklet context in these drivers is what the callbacks
from the dma client device is run in, and a lot of these probably
want something other than tasklet context, e.g. just call
complete() on a client-provided completion structure.

Instead of open-coding the use of the system_bh_wq in each
dmaengine, how about we start with a custom WQ_BH
specifically for the dmaengine subsystem and wrap them
inside of another interface.

Since almost every driver associates the tasklet with the
dma_chan, we could go one step further and add the
work_queue structure directly into struct dma_chan,
with the wrapper operating on the dma_chan rather than
the work_queue.

  Arnd


Re: [PATCH 2/9] dma: Convert from tasklet to BH workqueue

2024-03-28 Thread Vinod Koul
Hi Allen,

Subsytem is dmaengine, can you rename this to dmaengine: ...

On 27-03-24, 16:03, Allen Pais wrote:
> The only generic interface to execute asynchronously in the BH context is
> tasklet; however, it's marked deprecated and has some design flaws. To
> replace tasklets, BH workqueue support was recently added. A BH workqueue
> behaves similarly to regular workqueues except that the queued work items
> are executed in the BH context.

Thanks for conversion, am happy with BH alternative as it helps in
dmaengine where we need shortest possible time between tasklet and
interrupt handling to maximize dma performance

> 
> This patch converts drivers/dma/* from tasklet to BH workqueue.

> 
> Based on the work done by Tejun Heo 
> Branch: git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git for-6.10
> 
> Signed-off-by: Allen Pais 
> ---
>  drivers/dma/altera-msgdma.c   | 15 
>  drivers/dma/apple-admac.c | 15 
>  drivers/dma/at_hdmac.c|  2 +-
>  drivers/dma/at_xdmac.c| 15 
>  drivers/dma/bcm2835-dma.c |  2 +-
>  drivers/dma/dma-axi-dmac.c|  2 +-
>  drivers/dma/dma-jz4780.c  |  2 +-
>  .../dma/dw-axi-dmac/dw-axi-dmac-platform.c|  2 +-
>  drivers/dma/dw-edma/dw-edma-core.c|  2 +-
>  drivers/dma/dw/core.c | 13 +++
>  drivers/dma/dw/regs.h |  3 +-
>  drivers/dma/ep93xx_dma.c  | 15 
>  drivers/dma/fsl-edma-common.c |  2 +-
>  drivers/dma/fsl-qdma.c|  2 +-
>  drivers/dma/fsl_raid.c| 11 +++---
>  drivers/dma/fsl_raid.h|  2 +-
>  drivers/dma/fsldma.c  | 15 
>  drivers/dma/fsldma.h  |  3 +-
>  drivers/dma/hisi_dma.c|  2 +-
>  drivers/dma/hsu/hsu.c |  2 +-
>  drivers/dma/idma64.c  |  4 +--
>  drivers/dma/img-mdc-dma.c |  2 +-
>  drivers/dma/imx-dma.c | 27 +++---
>  drivers/dma/imx-sdma.c|  6 ++--
>  drivers/dma/ioat/dma.c| 17 -
>  drivers/dma/ioat/dma.h|  5 +--
>  drivers/dma/ioat/init.c   |  2 +-
>  drivers/dma/k3dma.c   | 19 +-
>  drivers/dma/mediatek/mtk-cqdma.c  | 35 ++-
>  drivers/dma/mediatek/mtk-hsdma.c  |  2 +-
>  drivers/dma/mediatek/mtk-uart-apdma.c |  4 +--
>  drivers/dma/mmp_pdma.c| 13 +++
>  drivers/dma/mmp_tdma.c| 11 +++---
>  drivers/dma/mpc512x_dma.c | 17 -
>  drivers/dma/mv_xor.c  | 13 +++
>  drivers/dma/mv_xor.h  |  5 +--
>  drivers/dma/mv_xor_v2.c   | 23 ++--
>  drivers/dma/mxs-dma.c | 13 +++
>  drivers/dma/nbpfaxi.c | 15 
>  drivers/dma/owl-dma.c |  2 +-
>  drivers/dma/pch_dma.c | 17 -
>  drivers/dma/pl330.c   | 31 
>  drivers/dma/plx_dma.c | 13 +++
>  drivers/dma/ppc4xx/adma.c | 17 -
>  drivers/dma/ppc4xx/adma.h |  5 +--
>  drivers/dma/pxa_dma.c |  2 +-
>  drivers/dma/qcom/bam_dma.c| 35 ++-
>  drivers/dma/qcom/gpi.c| 18 +-
>  drivers/dma/qcom/hidma.c  | 11 +++---
>  drivers/dma/qcom/hidma.h  |  5 +--
>  drivers/dma/qcom/hidma_ll.c   | 11 +++---
>  drivers/dma/qcom/qcom_adm.c   |  2 +-
>  drivers/dma/sa11x0-dma.c  | 27 +++---
>  drivers/dma/sf-pdma/sf-pdma.c | 23 ++--
>  drivers/dma/sf-pdma/sf-pdma.h |  5 +--
>  drivers/dma/sprd-dma.c|  2 +-
>  drivers/dma/st_fdma.c |  2 +-
>  drivers/dma/ste_dma40.c   | 17 -
>  drivers/dma/sun6i-dma.c   | 33 -
>  drivers/dma/tegra186-gpc-dma.c|  2 +-
>  drivers/dma/tegra20-apb-dma.c | 19 +-
>  drivers/dma/tegra210-adma.c   |  2 +-
>  drivers/dma/ti/edma.c |  2 +-
>  drivers/dma/ti/k3-udma.c  | 11 +++---
>  drivers/dma/ti/omap-dma.c |  2 +-
>  drivers/dma/timb_dma.c| 23 ++--
>  drivers/dma/txx9dmac.c| 29 +++
>  drivers/dma/txx9dmac.h  

Re: [PATCH v2 2/3] arch: Remove struct fb_info from video helpers

2024-03-28 Thread Thomas Zimmermann

Hi

Am 28.03.24 um 12:04 schrieb Helge Deller:

On 3/27/24 21:41, Thomas Zimmermann wrote:

The per-architecture video helpers do not depend on struct fb_info
or anything else from fbdev. Remove it from the interface and replace
fb_is_primary_device() with video_is_primary_device(). The new helper


Since you rename this function, wouldn't something similar to

device_is_primary_display()
or
device_is_primary_console()
or
is_primary_graphics_device()
or
is_primary_display_device()

be a better name?


The video_ prefix is there to signal that the code is part of the video 
subsystem.


But there's too much code that tried to figure out a default video 
device. So I actually have different plans for this function. I'd like 
to replace it with a helper that returns the default device instead of 
just testing for it. Sample code for x86 is already in vgaarb.c. [1] The 
function's name would then be video_default_device() and return the 
appropriate struct device*. video_is_primary device() will be removed. 
This rename here is the easiest step towards the new helper. Ok?


Best regards
Thomas

[1] https://elixir.bootlin.com/linux/v6.8/source/drivers/pci/vgaarb.c#L559



Helge


is similar in functionality, but can operate on non-fbdev devices.

Signed-off-by: Thomas Zimmermann 
Cc: "James E.J. Bottomley" 
Cc: Helge Deller 
Cc: "David S. Miller" 
Cc: Andreas Larsson 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: Borislav Petkov 
Cc: Dave Hansen 
Cc: x...@kernel.org
Cc: "H. Peter Anvin" 
---
  arch/parisc/include/asm/fb.h |  8 +---
  arch/parisc/video/fbdev.c    |  9 +
  arch/sparc/include/asm/fb.h  |  7 ---
  arch/sparc/video/fbdev.c | 17 -
  arch/x86/include/asm/fb.h    |  8 +---
  arch/x86/video/fbdev.c   | 18 +++---
  drivers/video/fbdev/core/fbcon.c |  2 +-
  include/asm-generic/fb.h | 11 ++-
  8 files changed, 41 insertions(+), 39 deletions(-)

diff --git a/arch/parisc/include/asm/fb.h b/arch/parisc/include/asm/fb.h
index 658a8a7dc5312..ed2a195a3e762 100644
--- a/arch/parisc/include/asm/fb.h
+++ b/arch/parisc/include/asm/fb.h
@@ -2,11 +2,13 @@
  #ifndef _ASM_FB_H_
  #define _ASM_FB_H_

-struct fb_info;
+#include 
+
+struct device;

  #if defined(CONFIG_STI_CORE)
-int fb_is_primary_device(struct fb_info *info);
-#define fb_is_primary_device fb_is_primary_device
+bool video_is_primary_device(struct device *dev);
+#define video_is_primary_device video_is_primary_device
  #endif

  #include 
diff --git a/arch/parisc/video/fbdev.c b/arch/parisc/video/fbdev.c
index e4f8ac99fc9e0..540fa0c919d59 100644
--- a/arch/parisc/video/fbdev.c
+++ b/arch/parisc/video/fbdev.c
@@ -5,12 +5,13 @@
   * Copyright (C) 2001-2002 Thomas Bogendoerfer 


   */

-#include 
  #include 

  #include 

-int fb_is_primary_device(struct fb_info *info)
+#include 
+
+bool video_is_primary_device(struct device *dev)
  {
  struct sti_struct *sti;

@@ -21,6 +22,6 @@ int fb_is_primary_device(struct fb_info *info)
  return true;

  /* return true if it's the default built-in framebuffer driver */
-    return (sti->dev == info->device);
+    return (sti->dev == dev);
  }
-EXPORT_SYMBOL(fb_is_primary_device);
+EXPORT_SYMBOL(video_is_primary_device);
diff --git a/arch/sparc/include/asm/fb.h b/arch/sparc/include/asm/fb.h
index 24440c0fda490..07f0325d6921c 100644
--- a/arch/sparc/include/asm/fb.h
+++ b/arch/sparc/include/asm/fb.h
@@ -3,10 +3,11 @@
  #define _SPARC_FB_H_

  #include 
+#include 

  #include 

-struct fb_info;
+struct device;

  #ifdef CONFIG_SPARC32
  static inline pgprot_t pgprot_framebuffer(pgprot_t prot,
@@ -18,8 +19,8 @@ static inline pgprot_t pgprot_framebuffer(pgprot_t 
prot,

  #define pgprot_framebuffer pgprot_framebuffer
  #endif

-int fb_is_primary_device(struct fb_info *info);
-#define fb_is_primary_device fb_is_primary_device
+bool video_is_primary_device(struct device *dev);
+#define video_is_primary_device video_is_primary_device

  static inline void fb_memcpy_fromio(void *to, const volatile void 
__iomem *from, size_t n)

  {
diff --git a/arch/sparc/video/fbdev.c b/arch/sparc/video/fbdev.c
index bff66dd1909a4..e46f0499c2774 100644
--- a/arch/sparc/video/fbdev.c
+++ b/arch/sparc/video/fbdev.c
@@ -1,26 +1,25 @@
  // SPDX-License-Identifier: GPL-2.0

  #include 
-#include 
+#include 
  #include 

+#include 
  #include 

-int fb_is_primary_device(struct fb_info *info)
+bool video_is_primary_device(struct device *dev)
  {
-    struct device *dev = info->device;
-    struct device_node *node;
+    struct device_node *node = dev->of_node;

  if (console_set_on_cmdline)
-    return 0;
+    return false;

-    node = dev->of_node;
  if (node && node == of_console_device)
-    return 1;
+    return true;

-    return 0;
+    return false;
  }
-EXPORT_SYMBOL(fb_is_primary_device);
+EXPORT_SYMBOL(video_is_primary_device);

  MODULE_DESCRIPTION("Sparc fbdev helpers");
  

Re: [PATCH v2 3/3] arch: Rename fbdev header and source files

2024-03-28 Thread Thomas Zimmermann

Hi

Am 28.03.24 um 13:51 schrieb Arnd Bergmann:

On Thu, Mar 28, 2024, at 13:46, Helge Deller wrote:

On 3/27/24 21:41, Thomas Zimmermann wrote:

+++ b/arch/arc/include/asm/video.h
@@ -0,0 +1,8 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _ASM_VIDEO_H_
+#define _ASM_VIDEO_H_
+
+#include 
+
+#endif /* _ASM_VIDEO_H_ */

I wonder, since that file simply #includes the generic version,
wasn't there a possibility that kbuild could symlink
the generic version for us?
Does it need to be mandatory in include/asm-generic/Kbuild ?
Same applies to a few other files below.

It should be enough to just remove the files entirely,
as kbuild will generate the same wrappers for mandatory files.


If that works, I'm happy to remove these wrapper files.

Best regards
Thomas



  Arnd


--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)



Re: [PATCH v2 1/3] arch: Select fbdev helpers with CONFIG_VIDEO

2024-03-28 Thread Thomas Zimmermann

Hi

Am 28.03.24 um 13:39 schrieb Helge Deller:

On 3/27/24 21:41, Thomas Zimmermann wrote:

Various Kconfig options selected the per-architecture helpers for
fbdev. But none of the contained code depends on fbdev. Standardize
on CONFIG_VIDEO, which will allow to add more general helpers for
video functionality.

CONFIG_VIDEO protects each architecture's video/ directory.


Your patch in general looks good.
But is renaming the config option from CONFIG_FB_CORE to CONFIG_VIDEO
the best choice?
CONFIG_VIDEO might be mixed up with multimedia/video-streaming.

Why not e.g. CONFIG_GRAPHICS_CORE?
I'm fine with CONFIG_VIDEO as well, but if someone has a better idea
we maybe should go with that instead now?


We already have CONFIG_VIDEO in drivers/video/Kconfig specifically for 
such low-level graphics code. For generic multimedia, we could have 
CONFIG_MEDIA, CONFIG_STREAMING, etc. rather than renaming an established 
Kconfig symbol.


Best regards
Thomas



Helge


This
allows for the use of more fine-grained control for each directory's
files, such as the use of CONFIG_STI_CORE on parisc.

v2:
- sparc: rebased onto Makefile changes

Signed-off-by: Thomas Zimmermann 
Cc: "James E.J. Bottomley" 
Cc: Helge Deller 
Cc: "David S. Miller" 
Cc: Andreas Larsson 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: Borislav Petkov 
Cc: Dave Hansen 
Cc: x...@kernel.org
Cc: "H. Peter Anvin" 
---
  arch/parisc/Makefile  | 2 +-
  arch/sparc/Makefile   | 4 ++--
  arch/sparc/video/Makefile | 2 +-
  arch/x86/Makefile | 2 +-
  arch/x86/video/Makefile   | 3 ++-
  5 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/arch/parisc/Makefile b/arch/parisc/Makefile
index 316f84f1d15c8..21b8166a68839 100644
--- a/arch/parisc/Makefile
+++ b/arch/parisc/Makefile
@@ -119,7 +119,7 @@ export LIBGCC

  libs-y    += arch/parisc/lib/ $(LIBGCC)

-drivers-y += arch/parisc/video/
+drivers-$(CONFIG_VIDEO) += arch/parisc/video/

  boot    := arch/parisc/boot

diff --git a/arch/sparc/Makefile b/arch/sparc/Makefile
index 2a03daa68f285..757451c3ea1df 100644
--- a/arch/sparc/Makefile
+++ b/arch/sparc/Makefile
@@ -59,8 +59,8 @@ endif
  libs-y += arch/sparc/prom/
  libs-y += arch/sparc/lib/

-drivers-$(CONFIG_PM) += arch/sparc/power/
-drivers-$(CONFIG_FB_CORE) += arch/sparc/video/
+drivers-$(CONFIG_PM)    += arch/sparc/power/
+drivers-$(CONFIG_VIDEO) += arch/sparc/video/

  boot := arch/sparc/boot

diff --git a/arch/sparc/video/Makefile b/arch/sparc/video/Makefile
index d4d83f1702c61..9dd82880a027a 100644
--- a/arch/sparc/video/Makefile
+++ b/arch/sparc/video/Makefile
@@ -1,3 +1,3 @@
  # SPDX-License-Identifier: GPL-2.0-only

-obj-$(CONFIG_FB_CORE) += fbdev.o
+obj-y    += fbdev.o
diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 15a5f4f2ff0aa..c0ea612c62ebe 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -265,7 +265,7 @@ drivers-$(CONFIG_PCI)    += arch/x86/pci/
  # suspend and hibernation support
  drivers-$(CONFIG_PM) += arch/x86/power/

-drivers-$(CONFIG_FB_CORE) += arch/x86/video/
+drivers-$(CONFIG_VIDEO) += arch/x86/video/

  
  # boot loader support. Several targets are kept for legacy purposes
diff --git a/arch/x86/video/Makefile b/arch/x86/video/Makefile
index 5ebe48752ffc4..9dd82880a027a 100644
--- a/arch/x86/video/Makefile
+++ b/arch/x86/video/Makefile
@@ -1,2 +1,3 @@
  # SPDX-License-Identifier: GPL-2.0-only
-obj-$(CONFIG_FB_CORE)    += fbdev.o
+
+obj-y    += fbdev.o




--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)



Re: [PATCH v2 3/3] arch: Rename fbdev header and source files

2024-03-28 Thread kernel test robot
Hi Thomas,

kernel test robot noticed the following build errors:

[auto build test ERROR on linus/master]
[also build test ERROR on v6.9-rc1 next-20240328]
[cannot apply to tip/x86/core deller-parisc/for-next arnd-asm-generic/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:
https://github.com/intel-lab-lkp/linux/commits/Thomas-Zimmermann/arch-Select-fbdev-helpers-with-CONFIG_VIDEO/20240328-044735
base:   linus/master
patch link:
https://lore.kernel.org/r/20240327204450.14914-4-tzimmermann%40suse.de
patch subject: [PATCH v2 3/3] arch: Rename fbdev header and source files
config: um-randconfig-001-20240328 
(https://download.01.org/0day-ci/archive/20240328/202403282102.oekobt3h-...@intel.com/config)
compiler: gcc-12 (Ubuntu 12.3.0-9ubuntu2) 12.3.0
reproduce (this is a W=1 build): 
(https://download.01.org/0day-ci/archive/20240328/202403282102.oekobt3h-...@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot 
| Closes: 
https://lore.kernel.org/oe-kbuild-all/202403282102.oekobt3h-...@intel.com/

All errors (new ones prefixed by >>):

   /usr/bin/ld: drivers/video/fbdev/core/fb_io_fops.o: in function `fb_io_mmap':
>> fb_io_fops.c:(.text+0x251): undefined reference to `pgprot_framebuffer'
   collect2: error: ld returned 1 exit status

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki


[PATCH] powerpc/crypto/chacha-p10: Fix failure on non Power10

2024-03-28 Thread Michael Ellerman
The chacha-p10-crypto module provides optimised chacha routines for
Power10. It also selects CRYPTO_ARCH_HAVE_LIB_CHACHA which says it
provides chacha_crypt_arch() to generic code.

Notably the module needs to provide chacha_crypt_arch() regardless of
whether it is loaded on Power10 or an older CPU.

The implementation of chacha_crypt_arch() already has a fallback to
chacha_crypt_generic(), however the module as a whole fails to load on
pre-Power10, because of the use of module_cpu_feature_match().

This breaks for example loading wireguard:

  jostaberry-1:~ # modprobe -v wireguard
  insmod 
/lib/modules/6.8.0-lp155.8.g7e0e887-default/kernel/arch/powerpc/crypto/chacha-p10-crypto.ko.zst
  modprobe: ERROR: could not insert 'wireguard': No such device

Fix it by removing module_cpu_feature_match(), and instead check the
CPU feature manually. If the CPU feature is not found, the module
still loads successfully, but doesn't register the Power10 specific
algorithms. That allows chacha_crypt_generic() to remain available for
use, fixing the problem.

  [root@fedora ~]# modprobe -v wireguard
  insmod /lib/modules/6.8.0-1-g786a790c4d79/kernel/net/ipv4/udp_tunnel.ko
  insmod 
/lib/modules/6.8.0-1-g786a790c4d79/kernel/net/ipv6/ip6_udp_tunnel.ko
  insmod /lib/modules/6.8.0-1-g786a790c4d79/kernel/lib/crypto/libchacha.ko
  insmod 
/lib/modules/6.8.0-1-g786a790c4d79/kernel/arch/powerpc/crypto/chacha-p10-crypto.ko
  insmod 
/lib/modules/6.8.0-1-g786a790c4d79/kernel/lib/crypto/libchacha20poly1305.ko
  insmod 
/lib/modules/6.8.0-1-g786a790c4d79/kernel/drivers/net/wireguard/wireguard.ko
  [   18.910452][  T721] wireguard: allowedips self-tests: pass
  [   18.914999][  T721] wireguard: nonce counter self-tests: pass
  [   19.029066][  T721] wireguard: ratelimiter self-tests: pass
  [   19.029257][  T721] wireguard: WireGuard 1.0.0 loaded. See 
www.wireguard.com for information.
  [   19.029361][  T721] wireguard: Copyright (C) 2015-2019 Jason A. Donenfeld 
. All Rights Reserved.

Reported-by: Michal Suchánek 
Closes: https://lore.kernel.org/all/20240315122005.gg20...@kitsune.suse.cz/
Signed-off-by: Michael Ellerman 
---
 arch/powerpc/crypto/chacha-p10-glue.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/crypto/chacha-p10-glue.c 
b/arch/powerpc/crypto/chacha-p10-glue.c
index 74fb86b0d209..7c728755852e 100644
--- a/arch/powerpc/crypto/chacha-p10-glue.c
+++ b/arch/powerpc/crypto/chacha-p10-glue.c
@@ -197,6 +197,9 @@ static struct skcipher_alg algs[] = {
 
 static int __init chacha_p10_init(void)
 {
+   if (!cpu_has_feature(CPU_FTR_ARCH_31))
+   return 0;
+
static_branch_enable(_p10);
 
return crypto_register_skciphers(algs, ARRAY_SIZE(algs));
@@ -204,10 +207,13 @@ static int __init chacha_p10_init(void)
 
 static void __exit chacha_p10_exit(void)
 {
+   if (!static_branch_likely(_p10))
+   return;
+
crypto_unregister_skciphers(algs, ARRAY_SIZE(algs));
 }
 
-module_cpu_feature_match(PPC_MODULE_FEATURE_P10, chacha_p10_init);
+module_init(chacha_p10_init);
 module_exit(chacha_p10_exit);
 
 MODULE_DESCRIPTION("ChaCha and XChaCha stream ciphers (P10 accelerated)");
-- 
2.44.0



Re: [PATCH v2 3/3] arch: Rename fbdev header and source files

2024-03-28 Thread Arnd Bergmann
On Thu, Mar 28, 2024, at 13:46, Helge Deller wrote:
> On 3/27/24 21:41, Thomas Zimmermann wrote:

>> +++ b/arch/arc/include/asm/video.h
>> @@ -0,0 +1,8 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +
>> +#ifndef _ASM_VIDEO_H_
>> +#define _ASM_VIDEO_H_
>> +
>> +#include 
>> +
>> +#endif /* _ASM_VIDEO_H_ */
>
> I wonder, since that file simply #includes the generic version,
> wasn't there a possibility that kbuild could symlink
> the generic version for us?
> Does it need to be mandatory in include/asm-generic/Kbuild ?
> Same applies to a few other files below.

It should be enough to just remove the files entirely,
as kbuild will generate the same wrappers for mandatory files.

 Arnd


Re: [PATCH v2 3/3] arch: Rename fbdev header and source files

2024-03-28 Thread Helge Deller

On 3/27/24 21:41, Thomas Zimmermann wrote:

The per-architecture fbdev code has no dependencies on fbdev and can
be used for any video-related subsystem. Rename the files to 'video'.
Use video-sti.c on parisc as the source file depends on CONFIG_STI_CORE.

Further update all includes statements, includ guards, and Makefiles.
Also update a few strings and comments to refer to video instead of
fbdev.

Signed-off-by: Thomas Zimmermann 
Cc: Vineet Gupta 
Cc: Catalin Marinas 
Cc: Will Deacon 
Cc: Huacai Chen 
Cc: WANG Xuerui 
Cc: Geert Uytterhoeven 
Cc: Thomas Bogendoerfer 
Cc: "James E.J. Bottomley" 
Cc: Helge Deller 
Cc: Michael Ellerman 
Cc: Nicholas Piggin 
Cc: Yoshinori Sato 
Cc: Rich Felker 
Cc: John Paul Adrian Glaubitz 
Cc: "David S. Miller" 
Cc: Andreas Larsson 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: Borislav Petkov 
Cc: Dave Hansen 
Cc: x...@kernel.org
Cc: "H. Peter Anvin" 
---
  arch/arc/include/asm/fb.h|  8 
  arch/arc/include/asm/video.h |  8 
  arch/arm/include/asm/fb.h|  6 --
  arch/arm/include/asm/video.h |  6 ++
  arch/arm64/include/asm/fb.h  | 10 --
  arch/arm64/include/asm/video.h   | 10 ++
  arch/loongarch/include/asm/{fb.h => video.h} |  8 
  arch/m68k/include/asm/{fb.h => video.h}  |  8 
  arch/mips/include/asm/{fb.h => video.h}  | 12 ++--
  arch/parisc/include/asm/{fb.h => video.h}|  8 
  arch/parisc/video/Makefile   |  2 +-
  arch/parisc/video/{fbdev.c => video-sti.c}   |  2 +-
  arch/powerpc/include/asm/{fb.h => video.h}   |  8 
  arch/powerpc/kernel/pci-common.c |  2 +-
  arch/sh/include/asm/fb.h |  7 ---
  arch/sh/include/asm/video.h  |  7 +++
  arch/sparc/include/asm/{fb.h => video.h} |  8 
  arch/sparc/video/Makefile|  2 +-
  arch/sparc/video/{fbdev.c => video.c}|  4 ++--
  arch/x86/include/asm/{fb.h => video.h}   |  8 
  arch/x86/video/Makefile  |  2 +-
  arch/x86/video/{fbdev.c => video.c}  |  3 ++-
  include/asm-generic/Kbuild   |  2 +-
  include/asm-generic/{fb.h => video.h}|  6 +++---
  include/linux/fb.h   |  2 +-
  25 files changed, 75 insertions(+), 74 deletions(-)
  delete mode 100644 arch/arc/include/asm/fb.h
  create mode 100644 arch/arc/include/asm/video.h
  delete mode 100644 arch/arm/include/asm/fb.h
  create mode 100644 arch/arm/include/asm/video.h
  delete mode 100644 arch/arm64/include/asm/fb.h
  create mode 100644 arch/arm64/include/asm/video.h
  rename arch/loongarch/include/asm/{fb.h => video.h} (86%)
  rename arch/m68k/include/asm/{fb.h => video.h} (86%)
  rename arch/mips/include/asm/{fb.h => video.h} (76%)
  rename arch/parisc/include/asm/{fb.h => video.h} (68%)
  rename arch/parisc/video/{fbdev.c => video-sti.c} (96%)
  rename arch/powerpc/include/asm/{fb.h => video.h} (76%)
  delete mode 100644 arch/sh/include/asm/fb.h
  create mode 100644 arch/sh/include/asm/video.h
  rename arch/sparc/include/asm/{fb.h => video.h} (89%)
  rename arch/sparc/video/{fbdev.c => video.c} (86%)
  rename arch/x86/include/asm/{fb.h => video.h} (77%)
  rename arch/x86/video/{fbdev.c => video.c} (97%)
  rename include/asm-generic/{fb.h => video.h} (96%)

diff --git a/arch/arc/include/asm/fb.h b/arch/arc/include/asm/fb.h
deleted file mode 100644
index 9c2383d29cbb9..0
--- a/arch/arc/include/asm/fb.h
+++ /dev/null
@@ -1,8 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-
-#ifndef _ASM_FB_H_
-#define _ASM_FB_H_
-
-#include 
-
-#endif /* _ASM_FB_H_ */
diff --git a/arch/arc/include/asm/video.h b/arch/arc/include/asm/video.h
new file mode 100644
index 0..8ff7263727fe7
--- /dev/null
+++ b/arch/arc/include/asm/video.h
@@ -0,0 +1,8 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _ASM_VIDEO_H_
+#define _ASM_VIDEO_H_
+
+#include 
+
+#endif /* _ASM_VIDEO_H_ */


I wonder, since that file simply #includes the generic version,
wasn't there a possibility that kbuild could symlink
the generic version for us?
Does it need to be mandatory in include/asm-generic/Kbuild ?
Same applies to a few other files below.

Helge




diff --git a/arch/arm/include/asm/fb.h b/arch/arm/include/asm/fb.h
deleted file mode 100644
index ce20a43c30339..0
--- a/arch/arm/include/asm/fb.h
+++ /dev/null
@@ -1,6 +0,0 @@
-#ifndef _ASM_FB_H_
-#define _ASM_FB_H_
-
-#include 
-
-#endif /* _ASM_FB_H_ */
diff --git a/arch/arm/include/asm/video.h b/arch/arm/include/asm/video.h
new file mode 100644
index 0..f570565366e67
--- /dev/null
+++ b/arch/arm/include/asm/video.h
@@ -0,0 +1,6 @@
+#ifndef _ASM_VIDEO_H_
+#define _ASM_VIDEO_H_
+
+#include 
+
+#endif /* _ASM_VIDEO_H_ */
diff --git a/arch/arm64/include/asm/fb.h b/arch/arm64/include/asm/fb.h
deleted file mode 100644
index 

Re: [PATCH v2 1/3] arch: Select fbdev helpers with CONFIG_VIDEO

2024-03-28 Thread Helge Deller

On 3/27/24 21:41, Thomas Zimmermann wrote:

Various Kconfig options selected the per-architecture helpers for
fbdev. But none of the contained code depends on fbdev. Standardize
on CONFIG_VIDEO, which will allow to add more general helpers for
video functionality.

CONFIG_VIDEO protects each architecture's video/ directory.


Your patch in general looks good.
But is renaming the config option from CONFIG_FB_CORE to CONFIG_VIDEO
the best choice?
CONFIG_VIDEO might be mixed up with multimedia/video-streaming.

Why not e.g. CONFIG_GRAPHICS_CORE?
I'm fine with CONFIG_VIDEO as well, but if someone has a better idea
we maybe should go with that instead now?

Helge


This
allows for the use of more fine-grained control for each directory's
files, such as the use of CONFIG_STI_CORE on parisc.

v2:
- sparc: rebased onto Makefile changes

Signed-off-by: Thomas Zimmermann 
Cc: "James E.J. Bottomley" 
Cc: Helge Deller 
Cc: "David S. Miller" 
Cc: Andreas Larsson 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: Borislav Petkov 
Cc: Dave Hansen 
Cc: x...@kernel.org
Cc: "H. Peter Anvin" 
---
  arch/parisc/Makefile  | 2 +-
  arch/sparc/Makefile   | 4 ++--
  arch/sparc/video/Makefile | 2 +-
  arch/x86/Makefile | 2 +-
  arch/x86/video/Makefile   | 3 ++-
  5 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/arch/parisc/Makefile b/arch/parisc/Makefile
index 316f84f1d15c8..21b8166a68839 100644
--- a/arch/parisc/Makefile
+++ b/arch/parisc/Makefile
@@ -119,7 +119,7 @@ export LIBGCC

  libs-y+= arch/parisc/lib/ $(LIBGCC)

-drivers-y += arch/parisc/video/
+drivers-$(CONFIG_VIDEO) += arch/parisc/video/

  boot  := arch/parisc/boot

diff --git a/arch/sparc/Makefile b/arch/sparc/Makefile
index 2a03daa68f285..757451c3ea1df 100644
--- a/arch/sparc/Makefile
+++ b/arch/sparc/Makefile
@@ -59,8 +59,8 @@ endif
  libs-y += arch/sparc/prom/
  libs-y += arch/sparc/lib/

-drivers-$(CONFIG_PM) += arch/sparc/power/
-drivers-$(CONFIG_FB_CORE) += arch/sparc/video/
+drivers-$(CONFIG_PM)+= arch/sparc/power/
+drivers-$(CONFIG_VIDEO) += arch/sparc/video/

  boot := arch/sparc/boot

diff --git a/arch/sparc/video/Makefile b/arch/sparc/video/Makefile
index d4d83f1702c61..9dd82880a027a 100644
--- a/arch/sparc/video/Makefile
+++ b/arch/sparc/video/Makefile
@@ -1,3 +1,3 @@
  # SPDX-License-Identifier: GPL-2.0-only

-obj-$(CONFIG_FB_CORE) += fbdev.o
+obj-y  += fbdev.o
diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 15a5f4f2ff0aa..c0ea612c62ebe 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -265,7 +265,7 @@ drivers-$(CONFIG_PCI)+= arch/x86/pci/
  # suspend and hibernation support
  drivers-$(CONFIG_PM) += arch/x86/power/

-drivers-$(CONFIG_FB_CORE) += arch/x86/video/
+drivers-$(CONFIG_VIDEO) += arch/x86/video/

  
  # boot loader support. Several targets are kept for legacy purposes
diff --git a/arch/x86/video/Makefile b/arch/x86/video/Makefile
index 5ebe48752ffc4..9dd82880a027a 100644
--- a/arch/x86/video/Makefile
+++ b/arch/x86/video/Makefile
@@ -1,2 +1,3 @@
  # SPDX-License-Identifier: GPL-2.0-only
-obj-$(CONFIG_FB_CORE)  += fbdev.o
+
+obj-y  += fbdev.o




Re: [PATCH v2 2/3] arch: Remove struct fb_info from video helpers

2024-03-28 Thread Helge Deller

On 3/27/24 21:41, Thomas Zimmermann wrote:

The per-architecture video helpers do not depend on struct fb_info
or anything else from fbdev. Remove it from the interface and replace
fb_is_primary_device() with video_is_primary_device(). The new helper


Since you rename this function, wouldn't something similar to

device_is_primary_display()
or
device_is_primary_console()
or
is_primary_graphics_device()
or
is_primary_display_device()

be a better name?

Helge


is similar in functionality, but can operate on non-fbdev devices.

Signed-off-by: Thomas Zimmermann 
Cc: "James E.J. Bottomley" 
Cc: Helge Deller 
Cc: "David S. Miller" 
Cc: Andreas Larsson 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: Borislav Petkov 
Cc: Dave Hansen 
Cc: x...@kernel.org
Cc: "H. Peter Anvin" 
---
  arch/parisc/include/asm/fb.h |  8 +---
  arch/parisc/video/fbdev.c|  9 +
  arch/sparc/include/asm/fb.h  |  7 ---
  arch/sparc/video/fbdev.c | 17 -
  arch/x86/include/asm/fb.h|  8 +---
  arch/x86/video/fbdev.c   | 18 +++---
  drivers/video/fbdev/core/fbcon.c |  2 +-
  include/asm-generic/fb.h | 11 ++-
  8 files changed, 41 insertions(+), 39 deletions(-)

diff --git a/arch/parisc/include/asm/fb.h b/arch/parisc/include/asm/fb.h
index 658a8a7dc5312..ed2a195a3e762 100644
--- a/arch/parisc/include/asm/fb.h
+++ b/arch/parisc/include/asm/fb.h
@@ -2,11 +2,13 @@
  #ifndef _ASM_FB_H_
  #define _ASM_FB_H_

-struct fb_info;
+#include 
+
+struct device;

  #if defined(CONFIG_STI_CORE)
-int fb_is_primary_device(struct fb_info *info);
-#define fb_is_primary_device fb_is_primary_device
+bool video_is_primary_device(struct device *dev);
+#define video_is_primary_device video_is_primary_device
  #endif

  #include 
diff --git a/arch/parisc/video/fbdev.c b/arch/parisc/video/fbdev.c
index e4f8ac99fc9e0..540fa0c919d59 100644
--- a/arch/parisc/video/fbdev.c
+++ b/arch/parisc/video/fbdev.c
@@ -5,12 +5,13 @@
   * Copyright (C) 2001-2002 Thomas Bogendoerfer 
   */

-#include 
  #include 

  #include 

-int fb_is_primary_device(struct fb_info *info)
+#include 
+
+bool video_is_primary_device(struct device *dev)
  {
struct sti_struct *sti;

@@ -21,6 +22,6 @@ int fb_is_primary_device(struct fb_info *info)
return true;

/* return true if it's the default built-in framebuffer driver */
-   return (sti->dev == info->device);
+   return (sti->dev == dev);
  }
-EXPORT_SYMBOL(fb_is_primary_device);
+EXPORT_SYMBOL(video_is_primary_device);
diff --git a/arch/sparc/include/asm/fb.h b/arch/sparc/include/asm/fb.h
index 24440c0fda490..07f0325d6921c 100644
--- a/arch/sparc/include/asm/fb.h
+++ b/arch/sparc/include/asm/fb.h
@@ -3,10 +3,11 @@
  #define _SPARC_FB_H_

  #include 
+#include 

  #include 

-struct fb_info;
+struct device;

  #ifdef CONFIG_SPARC32
  static inline pgprot_t pgprot_framebuffer(pgprot_t prot,
@@ -18,8 +19,8 @@ static inline pgprot_t pgprot_framebuffer(pgprot_t prot,
  #define pgprot_framebuffer pgprot_framebuffer
  #endif

-int fb_is_primary_device(struct fb_info *info);
-#define fb_is_primary_device fb_is_primary_device
+bool video_is_primary_device(struct device *dev);
+#define video_is_primary_device video_is_primary_device

  static inline void fb_memcpy_fromio(void *to, const volatile void __iomem 
*from, size_t n)
  {
diff --git a/arch/sparc/video/fbdev.c b/arch/sparc/video/fbdev.c
index bff66dd1909a4..e46f0499c2774 100644
--- a/arch/sparc/video/fbdev.c
+++ b/arch/sparc/video/fbdev.c
@@ -1,26 +1,25 @@
  // SPDX-License-Identifier: GPL-2.0

  #include 
-#include 
+#include 
  #include 

+#include 
  #include 

-int fb_is_primary_device(struct fb_info *info)
+bool video_is_primary_device(struct device *dev)
  {
-   struct device *dev = info->device;
-   struct device_node *node;
+   struct device_node *node = dev->of_node;

if (console_set_on_cmdline)
-   return 0;
+   return false;

-   node = dev->of_node;
if (node && node == of_console_device)
-   return 1;
+   return true;

-   return 0;
+   return false;
  }
-EXPORT_SYMBOL(fb_is_primary_device);
+EXPORT_SYMBOL(video_is_primary_device);

  MODULE_DESCRIPTION("Sparc fbdev helpers");
  MODULE_LICENSE("GPL");
diff --git a/arch/x86/include/asm/fb.h b/arch/x86/include/asm/fb.h
index c3b9582de7efd..999db33792869 100644
--- a/arch/x86/include/asm/fb.h
+++ b/arch/x86/include/asm/fb.h
@@ -2,17 +2,19 @@
  #ifndef _ASM_X86_FB_H
  #define _ASM_X86_FB_H

+#include 
+
  #include 

-struct fb_info;
+struct device;

  pgprot_t pgprot_framebuffer(pgprot_t prot,
unsigned long vm_start, unsigned long vm_end,
unsigned long offset);
  #define pgprot_framebuffer pgprot_framebuffer

-int fb_is_primary_device(struct fb_info *info);
-#define fb_is_primary_device fb_is_primary_device
+bool 

Re: [PATCH][next] crypto/nx: Avoid -Wflex-array-member-not-at-end warning

2024-03-28 Thread Herbert Xu
On Tue, Mar 05, 2024 at 01:57:56PM -0600, Gustavo A. R. Silva wrote:
> -Wflex-array-member-not-at-end is coming in GCC-14, and we are getting
> ready to enable it globally. So, we are deprecating flexible-array
> members in the middle of another structure.
> 
> There is currently an object (`header`) in `struct nx842_crypto_ctx`
> that contains a flexible structure (`struct nx842_crypto_header`):
> 
> struct nx842_crypto_ctx {
>   ...
> struct nx842_crypto_header header;
> struct nx842_crypto_header_group group[NX842_CRYPTO_GROUP_MAX];
>   ...
> };
> 
> So, in order to avoid ending up with a flexible-array member in the
> middle of another struct, we use the `struct_group_tagged()` helper to
> separate the flexible array from the rest of the members in the flexible
> structure:
> 
> struct nx842_crypto_header {
>   struct_group_tagged(nx842_crypto_header_hdr, hdr,
> 
>   ... the rest of the members
> 
>   );
> struct nx842_crypto_header_group group[];
> } __packed;
> 
> With the change described above, we can now declare an object of the
> type of the tagged struct, without embedding the flexible array in the
> middle of another struct:
> 
> struct nx842_crypto_ctx {
>   ...
> struct nx842_crypto_header_hdr header;
> struct nx842_crypto_header_group group[NX842_CRYPTO_GROUP_MAX];
>   ...
>  } __packed;
> 
> We also use `container_of()` whenever we need to retrieve a pointer to
> the flexible structure, through which we can access the flexible
> array if needed.
> 
> So, with these changes, fix the following warning:
> 
> In file included from drivers/crypto/nx/nx-842.c:55:
> drivers/crypto/nx/nx-842.h:174:36: warning: structure containing a flexible 
> array member is not at the end of another structure 
> [-Wflex-array-member-not-at-end]
>   174 | struct nx842_crypto_header header;
>   |^~
> 
> Signed-off-by: Gustavo A. R. Silva 
> ---
>  drivers/crypto/nx/nx-842.c |  6 --
>  drivers/crypto/nx/nx-842.h | 10 ++
>  2 files changed, 10 insertions(+), 6 deletions(-)

Patch applied.  Thanks.
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt


Re: [PATCH v2 3/3] arch: Rename fbdev header and source files

2024-03-28 Thread Sam Ravnborg
Hi Thomas,

On Wed, Mar 27, 2024 at 09:41:31PM +0100, Thomas Zimmermann wrote:
> The per-architecture fbdev code has no dependencies on fbdev and can
> be used for any video-related subsystem. Rename the files to 'video'.
> Use video-sti.c on parisc as the source file depends on CONFIG_STI_CORE.
> 
> Further update all includes statements, includ guards, and Makefiles.
^ missing 'e' 

> Also update a few strings and comments to refer to video instead of
> fbdev.
> 
> Signed-off-by: Thomas Zimmermann 
> Cc: Vineet Gupta 
> Cc: Catalin Marinas 
> Cc: Will Deacon 
> Cc: Huacai Chen 
> Cc: WANG Xuerui 
> Cc: Geert Uytterhoeven 
> Cc: Thomas Bogendoerfer 
> Cc: "James E.J. Bottomley" 
> Cc: Helge Deller 
> Cc: Michael Ellerman 
> Cc: Nicholas Piggin 
> Cc: Yoshinori Sato 
> Cc: Rich Felker 
> Cc: John Paul Adrian Glaubitz 
> Cc: "David S. Miller" 
> Cc: Andreas Larsson 
> Cc: Thomas Gleixner 
> Cc: Ingo Molnar 
> Cc: Borislav Petkov 
> Cc: Dave Hansen 
> Cc: x...@kernel.org
> Cc: "H. Peter Anvin" 

If the patch is changed to use the Kbuild file to pick the generic
variant of video.h then it is:

Reviewed-by: Sam Ravnborg 

I also added an unrelated sparc comment, that can be addressed another
time.

Sam

> ---
>  arch/arc/include/asm/fb.h|  8 
>  arch/arc/include/asm/video.h |  8 
>  arch/arm/include/asm/fb.h|  6 --
>  arch/arm/include/asm/video.h |  6 ++
>  arch/arm64/include/asm/fb.h  | 10 --
>  arch/arm64/include/asm/video.h   | 10 ++
>  arch/loongarch/include/asm/{fb.h => video.h} |  8 
>  arch/m68k/include/asm/{fb.h => video.h}  |  8 
>  arch/mips/include/asm/{fb.h => video.h}  | 12 ++--
>  arch/parisc/include/asm/{fb.h => video.h}|  8 
>  arch/parisc/video/Makefile   |  2 +-
>  arch/parisc/video/{fbdev.c => video-sti.c}   |  2 +-
>  arch/powerpc/include/asm/{fb.h => video.h}   |  8 
>  arch/powerpc/kernel/pci-common.c |  2 +-
>  arch/sh/include/asm/fb.h |  7 ---
>  arch/sh/include/asm/video.h  |  7 +++
>  arch/sparc/include/asm/{fb.h => video.h} |  8 
>  arch/sparc/video/Makefile|  2 +-
>  arch/sparc/video/{fbdev.c => video.c}|  4 ++--
>  arch/x86/include/asm/{fb.h => video.h}   |  8 
>  arch/x86/video/Makefile  |  2 +-
>  arch/x86/video/{fbdev.c => video.c}  |  3 ++-
>  include/asm-generic/Kbuild   |  2 +-
>  include/asm-generic/{fb.h => video.h}|  6 +++---
>  include/linux/fb.h   |  2 +-
>  25 files changed, 75 insertions(+), 74 deletions(-)
>  delete mode 100644 arch/arc/include/asm/fb.h
>  create mode 100644 arch/arc/include/asm/video.h
>  delete mode 100644 arch/arm/include/asm/fb.h
>  create mode 100644 arch/arm/include/asm/video.h
>  delete mode 100644 arch/arm64/include/asm/fb.h
>  create mode 100644 arch/arm64/include/asm/video.h
>  rename arch/loongarch/include/asm/{fb.h => video.h} (86%)
>  rename arch/m68k/include/asm/{fb.h => video.h} (86%)
>  rename arch/mips/include/asm/{fb.h => video.h} (76%)
>  rename arch/parisc/include/asm/{fb.h => video.h} (68%)
>  rename arch/parisc/video/{fbdev.c => video-sti.c} (96%)
>  rename arch/powerpc/include/asm/{fb.h => video.h} (76%)
>  delete mode 100644 arch/sh/include/asm/fb.h
>  create mode 100644 arch/sh/include/asm/video.h
>  rename arch/sparc/include/asm/{fb.h => video.h} (89%)
>  rename arch/sparc/video/{fbdev.c => video.c} (86%)
>  rename arch/x86/include/asm/{fb.h => video.h} (77%)
>  rename arch/x86/video/{fbdev.c => video.c} (97%)
>  rename include/asm-generic/{fb.h => video.h} (96%)
> 
> diff --git a/arch/arc/include/asm/fb.h b/arch/arc/include/asm/fb.h
> deleted file mode 100644
> index 9c2383d29cbb9..0
> --- a/arch/arc/include/asm/fb.h
> +++ /dev/null
> @@ -1,8 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0 */
> -
> -#ifndef _ASM_FB_H_
> -#define _ASM_FB_H_
> -
> -#include 
> -
> -#endif /* _ASM_FB_H_ */
> diff --git a/arch/arc/include/asm/video.h b/arch/arc/include/asm/video.h
> new file mode 100644
> index 0..8ff7263727fe7
> --- /dev/null
> +++ b/arch/arc/include/asm/video.h
> @@ -0,0 +1,8 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef _ASM_VIDEO_H_
> +#define _ASM_VIDEO_H_
> +
> +#include 
> +
> +#endif /* _ASM_VIDEO_H_ */

arch/arc/include/asm/video.h only exists to pick
include/asm-generic/video.h.

The simpler way to do this is to add a line to:
arch/arc/include/asm/Kbuild:
+ generic-y += video.h

While touching the file I suggest to move to the simpler way using the
Kbuild file.

The same goes for all the other video.h files that only picks the
asm-generic variant.




> diff --git a/arch/arm/include/asm/fb.h b/arch/arm/include/asm/fb.h
> deleted file mode 

Re: [PATCH v2 1/3] arch: Select fbdev helpers with CONFIG_VIDEO

2024-03-28 Thread Sam Ravnborg
On Wed, Mar 27, 2024 at 09:41:29PM +0100, Thomas Zimmermann wrote:
> Various Kconfig options selected the per-architecture helpers for
> fbdev. But none of the contained code depends on fbdev. Standardize
> on CONFIG_VIDEO, which will allow to add more general helpers for
> video functionality.
> 
> CONFIG_VIDEO protects each architecture's video/ directory. This
> allows for the use of more fine-grained control for each directory's
> files, such as the use of CONFIG_STI_CORE on parisc.
> 
> v2:
> - sparc: rebased onto Makefile changes
> 
> Signed-off-by: Thomas Zimmermann 
> Cc: "James E.J. Bottomley" 
> Cc: Helge Deller 
> Cc: "David S. Miller" 
> Cc: Andreas Larsson 
> Cc: Thomas Gleixner 
> Cc: Ingo Molnar 
> Cc: Borislav Petkov 
> Cc: Dave Hansen 
> Cc: x...@kernel.org
> Cc: "H. Peter Anvin" 

Reviewed-by: Sam Ravnborg 


Re: [PATCH v4 06/13] mm/gup: Drop folio_fast_pin_allowed() in hugepd processing

2024-03-28 Thread David Hildenbrand

On 27.03.24 16:23, pet...@redhat.com wrote:

From: Peter Xu 

Hugepd format for GUP is only used in PowerPC with hugetlbfs.  There are
some kernel usage of hugepd (can refer to hugepd_populate_kernel() for
PPC_8XX), however those pages are not candidates for GUP.

Commit a6e79df92e4a ("mm/gup: disallow FOLL_LONGTERM GUP-fast writing to
file-backed mappings") added a check to fail gup-fast if there's potential
risk of violating GUP over writeback file systems.  That should never apply
to hugepd.  Considering that hugepd is an old format (and even
software-only), there's no plan to extend hugepd into other file typed
memories that is prone to the same issue.

Drop that check, not only because it'll never be true for hugepd per any
known plan, but also it paves way for reusing the function outside
fast-gup.

To make sure we'll still remember this issue just in case hugepd will be
extended to support non-hugetlbfs memories, add a rich comment above
gup_huge_pd(), explaining the issue with proper references.

Cc: Christoph Hellwig 
Cc: Lorenzo Stoakes 
Cc: Michael Ellerman 
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Peter Xu 
---


@Andrew, you properly adjusted the code to remove the 
gup_fast_folio_allowed() call instead of the folio_fast_pin_allowed() 
call, but


(1) the commit subject
(2) comment for gup_huge_pd()

Still mention folio_fast_pin_allowed().

The patch "mm/gup: handle hugepd for follow_page()" then moves that 
(outdated) comment.


--
Cheers,

David / dhildenb



Re: Boot failure with ppc64 port on iMacs G5

2024-03-28 Thread John Paul Adrian Glaubitz
Hi Michael,

On Wed, 2024-03-06 at 12:57 +1100, Michael Ellerman wrote:
> > Yep, the second, older image works as expected. However, the recent one 
> > does not
> > and I have absolutely no clue why.
> 
> I actually tested both, and both work, but then I cited the wrong one in
> my email >_<
> 
> So at least on qemu that newer kernel is OK:
> 
>   Preparing to boot Linux version 6.6.15-powerpc64 
> (debian-ker...@lists.debian.org) (gcc-13 (Debian 13.2.0-13) 13.2.0, GNU ld 
> (GNU Binutils for Debian) 2.42) #1 SMP Debian 6.6.15-2 (2024-02-04)
>   ...
>   Booting Linux via __start() @ 0x0480 ...
>   Hello World !
>   smp_core99_probe
>   smp_core99_bringup_done
>   Starting system log daemon: syslogd, klogd.

Did you get around testing the images on real hardware?

And can tell me what command line you used for booting with QEMU?
Maybe that gives us a clue where the problem is.

Users are still reporting boot lockups with kernel 6.6.x.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913


Re: [PATCH v2 4/6] mm/mm_init.c: remove meaningless calculation of zone->managed_pages in free_area_init_core()

2024-03-28 Thread Mike Rapoport
On Thu, Mar 28, 2024 at 04:32:38PM +0800, Baoquan He wrote:
> On 03/25/24 at 10:56pm, Baoquan He wrote:
> >  
> > /*
> > -* Set an approximate value for lowmem here, it will be adjusted
> > -* when the bootmem allocator frees pages into the buddy system.
> > -* And all highmem pages will be managed by the buddy system.
> > +* Initialize zone->managed_pages as 0 , it will be reset
> > +* when memblock allocator frees pages into buddy system.
> >  */
> > -   zone_init_internals(zone, j, nid, freesize);
> > +   zone_init_internals(zone, j, nid, 0);
> 
> Here, we should initialize zone->managed_pages as zone->present_pages
> because later page_group_by_mobility_disabled need be set according to
> zone->managed_pages. Otherwise page_group_by_mobility_disabled will be
> set to 1 always. I will sent out v3.

With zone->managed_pages set to zone->present_pages we won't account for
the reserved memory for initialization of page_group_by_mobility_disabled.

As watermarks are still not initialized at the time build_all_zonelists()
is called, we may use nr_all_pages - nr_kernel_pages instead of
nr_free_zone_pages(), IMO.
 
> From a17b0921b4bd00596330f61ee9ea4b82386a9fed Mon Sep 17 00:00:00 2001
> From: Baoquan He 
> Date: Thu, 28 Mar 2024 16:20:15 +0800
> Subject: [PATCH] mm/mm_init.c: set zone's ->managed_pages as ->present_pages
>  for now
> Content-type: text/plain
> 
> Because page_group_by_mobility_disabled need be set according to zone's
> managed_pages later.
> 
> Signed-off-by: Baoquan He 
> ---
>  mm/mm_init.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/mm_init.c b/mm/mm_init.c
> index cc24e7958c0c..dd875f943cbb 100644
> --- a/mm/mm_init.c
> +++ b/mm/mm_init.c
> @@ -1561,7 +1561,7 @@ static void __init free_area_init_core(struct 
> pglist_data *pgdat)
>* Initialize zone->managed_pages as 0 , it will be reset
>* when memblock allocator frees pages into buddy system.
>*/
> - zone_init_internals(zone, j, nid, 0);
> + zone_init_internals(zone, j, nid, zone->present_pages);
>  
>   if (!size)
>   continue;
> -- 
> 2.41.0
> 
> 
> >  
> > if (!size)
> > continue;
> > @@ -1915,6 +1878,7 @@ void __init free_area_init(unsigned long 
> > *max_zone_pfn)
> > check_for_memory(pgdat);
> > }
> >  
> > +   calc_nr_kernel_pages();
> > memmap_init();
> >  
> > /* disable hash distribution for systems with a single node */
> > -- 
> > 2.41.0
> > 
> 

-- 
Sincerely yours,
Mike.


Re: [PATCH RFC 0/3] mm/gup: consistently call it GUP-fast

2024-03-28 Thread David Hildenbrand

On 28.03.24 08:15, Mike Rapoport wrote:

On Thu, Mar 28, 2024 at 07:09:13AM +0100, Arnd Bergmann wrote:

On Thu, Mar 28, 2024, at 06:51, Vineet Gupta wrote:

On 3/27/24 09:22, Arnd Bergmann wrote:

On Wed, Mar 27, 2024, at 16:39, David Hildenbrand wrote:

On 27.03.24 16:21, Peter Xu wrote:

On Wed, Mar 27, 2024 at 02:05:35PM +0100, David Hildenbrand wrote:

I'm not sure what config you tried there; as I am doing some build tests
recently, I found turning off CONFIG_SAMPLES + CONFIG_GCC_PLUGINS could
avoid a lot of issues, I think it's due to libc missing.  But maybe not the
case there.

CCin Arnd; I use some of his compiler chains, others from Fedora directly. For
example for alpha and arc, the Fedora gcc is "13.2.1".
But there is other stuff like (arc):

./arch/arc/include/asm/mmu-arcv2.h: In function 'mmu_setup_asid':
./arch/arc/include/asm/mmu-arcv2.h:82:9: error: implicit declaration of
function 'write_aux_reg' [-Werro
r=implicit-function-declaration]
 82 | write_aux_reg(ARC_REG_PID, asid | MMU_ENABLE);
| ^

Seems to be missing an #include of soc/arc/aux.h, but I can't
tell when this first broke without bisecting.


Weird I don't see this one but I only have gcc 12 handy ATM.

     gcc version 12.2.1 20230306 (ARC HS GNU/Linux glibc toolchain -
build 1360)

I even tried W=1 (which according to scripts/Makefile.extrawarn) should
include -Werror=implicit-function-declaration but don't see this still.

Tomorrow I'll try building a gcc 13.2.1 for ARC.


David reported them with the toolchains I built at
https://mirrors.edge.kernel.org/pub/tools/crosstool/
I'm fairly sure the problem is specific to the .config
and tree, not the toolchain though.


This happens with defconfig and both gcc 12.2.0 and gcc 13.2.0 from your
crosstools. I also see these on the current Linus' tree:

arc/kernel/ptrace.c:342:16: warning: no previous prototype for 
'syscall_trace_enter' [-Wmissing-prototypes]
arch/arc/kernel/kprobes.c:193:15: warning: no previous prototype for 
'arc_kprobe_handler' [-Wmissing-prototypes]

This fixed the warning about write_aux_reg for me, probably Vineet would
want this include somewhere else...

diff --git a/arch/arc/include/asm/mmu-arcv2.h b/arch/arc/include/asm/mmu-arcv2.h
index ed9036d4ede3..0fca342d7b79 100644
--- a/arch/arc/include/asm/mmu-arcv2.h
+++ b/arch/arc/include/asm/mmu-arcv2.h
@@ -69,6 +69,8 @@
  
  #ifndef __ASSEMBLY__
  
+#include 

+
  struct mm_struct;
  extern int pae40_exist_but_not_enab(void);



Here are all err+warn I see with my configs on Linus' tree from today (not 
mm-unstable).
Most of them are warnings due to missing prototypes or missing "clone3".

Parisc64 seems to be a bit more broken. Maybe nobody cares about parisc64 
anymore? Or
it's a toolchain issue, don't know.

xtensa is also broken, but "invalid register" smells like a toolchain issue to 
me.


Maybe all known/expected, just posting it if anybody cares. I can share my full 
build script
on request.



[INFO] Compiling alpha
[INFO] 0 errors
[INFO] 102 warnings
[PASS]

$ cat alpha_log  | grep warn
:1519:2: warning: #warning syscall clone3 not implemented [-Wcpp]
arch/alpha/lib/checksum.c:45:9: warning: no previous prototype for 
'csum_tcpudp_magic' [-Wmissing-prototypes]
arch/alpha/lib/checksum.c:54:8: warning: no previous prototype for 
'csum_tcpudp_nofold' [-Wmissing-prototypes]
arch/alpha/lib/checksum.c:145:9: warning: no previous prototype for 
'ip_fast_csum' [-Wmissing-prototypes]
arch/alpha/lib/checksum.c:163:8: warning: no previous prototype for 
'csum_partial' [-Wmissing-prototypes]
arch/alpha/lib/checksum.c:180:9: warning: no previous prototype for 
'ip_compute_csum' [-Wmissing-prototypes]
arch/alpha/kernel/traps.c:211:1: warning: no previous prototype for 
'do_entArith' [-Wmissing-prototypes]
arch/alpha/kernel/traps.c:233:1: warning: no previous prototype for 'do_entIF' 
[-Wmissing-prototypes]
arch/alpha/kernel/traps.c:400:1: warning: no previous prototype for 'do_entDbg' 
[-Wmissing-prototypes]
arch/alpha/kernel/traps.c:436:1: warning: no previous prototype for 'do_entUna' 
[-Wmissing-prototypes]
arch/alpha/kernel/traps.c:721:1: warning: no previous prototype for 
'do_entUnaUser' [-Wmissing-prototypes]
arch/alpha/mm/init.c:261:1: warning: no previous prototype for 
'srm_paging_stop' [-Wmissing-prototypes]
arch/alpha/lib/fpreg.c:20:1: warning: no previous prototype for 
'alpha_read_fp_reg' [-Wmissing-prototypes]
[]

[INFO] Compiling arc
[INFO] 0 errors
[INFO] 2 warnings
[PASS]

$ cat arc_log  | grep warn
arch/arc/kernel/ptrace.c:342:16: warning: no previous prototype for 
'syscall_trace_enter' [-Wmissing-prototypes]
arch/arc/kernel/kprobes.c:193:15: warning: no previous prototype for 
'arc_kprobe_handler' [-Wmissing-prototypes]


[INFO] Compiling hexagon
[INFO] 0 errors
[INFO] 1 warnings
[PASS]

 $ cat hexagon_log  | grep warn
:1519:2: warning: syscall clone3 not implemented [-W#warnings]
 1519 | #warning syscall clone3 not implemented
1 warning generated.


Re: [PATCH v11 00/11] Support page table check PowerPC

2024-03-28 Thread Ingo Molnar


* Rohan McLure  wrote:

> Rohan McLure (11):
>   Revert "mm/page_table_check: remove unused parameter in 
> [__]page_table_check_pud_set"
>   Revert "mm/page_table_check: remove unused parameter in 
> [__]page_table_check_pmd_set"
>   Revert "mm/page_table_check: remove unused parameter in 
> [__]page_table_check_pud_clear"
>   Revert "mm/page_table_check: remove unused parameter in 
> [__]page_table_check_pmd_clear"
>   Revert "mm/page_table_check: remove unused parameter in 
> [__]page_table_check_pte_clear"

Just a process request: please give these commits proper titles, they 
are not really 'reverts' in the classical sense, and this title hides 
what is being done in the commit. The typical use of reverts is to 
revert a bad change because it broke something. Here the goal is to 
reintroduce functionality.

So please name these 5 patches accordingly, to shed light on what is 
being reintroduced. You can mention it at the end of the changelog that 
it's a functional revert of commit XYZ, but that's not the primary 
purpose of the commit.

Thanks,

Ingo


[PATCH v3 4/6] mm/mm_init.c: remove meaningless calculation of zone->managed_pages in free_area_init_core()

2024-03-28 Thread Baoquan He
Currently, in free_area_init_core(), when initialize zone's field, a
rough value is set to zone->managed_pages. That value is calculated by
(zone->present_pages - memmap_pages).

In the meantime, add the value to nr_all_pages and nr_kernel_pages which
represent all free pages of system (only low memory or including HIGHMEM
memory separately). Both of them are gonna be used in
alloc_large_system_hash().

However, the rough calculation and setting of zone->managed_pages is
meaningless because
  a) memmap pages are allocated on units of node in sparse_init() or
 alloc_node_mem_map(pgdat); The simple (zone->present_pages -
 memmap_pages) is too rough to make sense for zone;
  b) the set zone->managed_pages will be zeroed out and reset with
 acutal value in mem_init() via memblock_free_all(). Before the
 resetting, no buddy allocation request is issued.

Here, remove the meaningless and complicated calculation of
(zone->present_pages - memmap_pages), directly set zone->managed_pages
as zone->present_pages for now. It will be adjusted in mem_init().

And also remove the assignment of nr_all_pages and nr_kernel_pages in
free_area_init_core(). Instead, call the newly added calc_nr_kernel_pages()
to count up all free but not reserved memory in memblock and assign to
nr_all_pages and nr_kernel_pages. The counting excludes memmap_pages,
and other kernel used data, which is more accurate than old way and
simpler, and can also cover the ppc required arch_reserved_kernel_pages()
case.

And also clean up the outdated code comment above free_area_init_core().
And free_area_init_core() is easy to understand now, no need to add
words to explain.

Signed-off-by: Baoquan He 
---
v2->v3:
- Change to initialize zone->managed_pages as zone->present_pages for now
  because later page_group_by_mobility_disabled need be set according to
  zone->managed_pages. Otherwise it will cause setting
  page_group_by_mobility_disabled to 1 always.

 mm/mm_init.c | 46 +-
 1 file changed, 5 insertions(+), 41 deletions(-)

diff --git a/mm/mm_init.c b/mm/mm_init.c
index c57a7fc97a16..a4b80e8276bb 100644
--- a/mm/mm_init.c
+++ b/mm/mm_init.c
@@ -1565,15 +1565,6 @@ void __ref free_area_init_core_hotplug(struct 
pglist_data *pgdat)
 }
 #endif
 
-/*
- * Set up the zone data structures:
- *   - mark all pages reserved
- *   - mark all memory queues empty
- *   - clear the memory bitmaps
- *
- * NOTE: pgdat should get zeroed by caller.
- * NOTE: this function is only called during early init.
- */
 static void __init free_area_init_core(struct pglist_data *pgdat)
 {
enum zone_type j;
@@ -1584,41 +1575,13 @@ static void __init free_area_init_core(struct 
pglist_data *pgdat)
 
for (j = 0; j < MAX_NR_ZONES; j++) {
struct zone *zone = pgdat->node_zones + j;
-   unsigned long size, freesize, memmap_pages;
-
-   size = zone->spanned_pages;
-   freesize = zone->present_pages;
-
-   /*
-* Adjust freesize so that it accounts for how much memory
-* is used by this zone for memmap. This affects the watermark
-* and per-cpu initialisations
-*/
-   memmap_pages = calc_memmap_size(size, freesize);
-   if (!is_highmem_idx(j)) {
-   if (freesize >= memmap_pages) {
-   freesize -= memmap_pages;
-   if (memmap_pages)
-   pr_debug("  %s zone: %lu pages used for 
memmap\n",
-zone_names[j], memmap_pages);
-   } else
-   pr_warn("  %s zone: %lu memmap pages exceeds 
freesize %lu\n",
-   zone_names[j], memmap_pages, freesize);
-   }
-
-   if (!is_highmem_idx(j))
-   nr_kernel_pages += freesize;
-   /* Charge for highmem memmap if there are enough kernel pages */
-   else if (nr_kernel_pages > memmap_pages * 2)
-   nr_kernel_pages -= memmap_pages;
-   nr_all_pages += freesize;
+   unsigned long size = zone->spanned_pages;
 
/*
-* Set an approximate value for lowmem here, it will be adjusted
-* when the bootmem allocator frees pages into the buddy system.
-* And all highmem pages will be managed by the buddy system.
+* Initialize zone->managed_pages as 0 , it will be reset
+* when memblock allocator frees pages into buddy system.
 */
-   zone_init_internals(zone, j, nid, freesize);
+   zone_init_internals(zone, j, nid, zone->present_pages);
 
if (!size)
continue;
@@ -1915,6 +1878,7 @@ void __init free_area_init(unsigned long 

Re: [PATCH v2 4/6] mm/mm_init.c: remove meaningless calculation of zone->managed_pages in free_area_init_core()

2024-03-28 Thread Baoquan He
On 03/25/24 at 10:56pm, Baoquan He wrote:
> Currently, in free_area_init_core(), when initialize zone's field, a
> rough value is set to zone->managed_pages. That value is calculated by
> (zone->present_pages - memmap_pages).
> 
> In the meantime, add the value to nr_all_pages and nr_kernel_pages which
> represent all free pages of system (only low memory or including HIGHMEM
> memory separately). Both of them are gonna be used in
> alloc_large_system_hash().
> 
> However, the rough calculation and setting of zone->managed_pages is
> meaningless because
>   a) memmap pages are allocated on units of node in sparse_init() or
>  alloc_node_mem_map(pgdat); The simple (zone->present_pages -
>  memmap_pages) is too rough to make sense for zone;
>   b) the set zone->managed_pages will be zeroed out and reset with
>  acutal value in mem_init() via memblock_free_all(). Before the
>  resetting, no buddy allocation request is issued.
> 
> Here, remove the meaningless and complicated calculation of
> (zone->present_pages - memmap_pages), initialize zone->managed_pages as 0
> which reflect its actual value because no any page is added into buddy
> system right now. It will be reset in mem_init().
> 
> And also remove the assignment of nr_all_pages and nr_kernel_pages in
> free_area_init_core(). Instead, call the newly added calc_nr_kernel_pages()
> to count up all free but not reserved memory in memblock and assign to
> nr_all_pages and nr_kernel_pages. The counting excludes memmap_pages,
> and other kernel used data, which is more accurate than old way and
> simpler, and can also cover the ppc required arch_reserved_kernel_pages()
> case.
> 
> And also clean up the outdated code comment above free_area_init_core().
> And free_area_init_core() is easy to understand now, no need to add
> words to explain.
> 
> Signed-off-by: Baoquan He 
> ---
>  mm/mm_init.c | 46 +-
>  1 file changed, 5 insertions(+), 41 deletions(-)
> 
> diff --git a/mm/mm_init.c b/mm/mm_init.c
> index c57a7fc97a16..7f71e56e83f3 100644
> --- a/mm/mm_init.c
> +++ b/mm/mm_init.c
> @@ -1565,15 +1565,6 @@ void __ref free_area_init_core_hotplug(struct 
> pglist_data *pgdat)
>  }
>  #endif
>  
> -/*
> - * Set up the zone data structures:
> - *   - mark all pages reserved
> - *   - mark all memory queues empty
> - *   - clear the memory bitmaps
> - *
> - * NOTE: pgdat should get zeroed by caller.
> - * NOTE: this function is only called during early init.
> - */
>  static void __init free_area_init_core(struct pglist_data *pgdat)
>  {
>   enum zone_type j;
> @@ -1584,41 +1575,13 @@ static void __init free_area_init_core(struct 
> pglist_data *pgdat)
>  
>   for (j = 0; j < MAX_NR_ZONES; j++) {
>   struct zone *zone = pgdat->node_zones + j;
> - unsigned long size, freesize, memmap_pages;
> -
> - size = zone->spanned_pages;
> - freesize = zone->present_pages;
> -
> - /*
> -  * Adjust freesize so that it accounts for how much memory
> -  * is used by this zone for memmap. This affects the watermark
> -  * and per-cpu initialisations
> -  */
> - memmap_pages = calc_memmap_size(size, freesize);
> - if (!is_highmem_idx(j)) {
> - if (freesize >= memmap_pages) {
> - freesize -= memmap_pages;
> - if (memmap_pages)
> - pr_debug("  %s zone: %lu pages used for 
> memmap\n",
> -  zone_names[j], memmap_pages);
> - } else
> - pr_warn("  %s zone: %lu memmap pages exceeds 
> freesize %lu\n",
> - zone_names[j], memmap_pages, freesize);
> - }
> -
> - if (!is_highmem_idx(j))
> - nr_kernel_pages += freesize;
> - /* Charge for highmem memmap if there are enough kernel pages */
> - else if (nr_kernel_pages > memmap_pages * 2)
> - nr_kernel_pages -= memmap_pages;
> - nr_all_pages += freesize;
> + unsigned long size = zone->spanned_pages;
>  
>   /*
> -  * Set an approximate value for lowmem here, it will be adjusted
> -  * when the bootmem allocator frees pages into the buddy system.
> -  * And all highmem pages will be managed by the buddy system.
> +  * Initialize zone->managed_pages as 0 , it will be reset
> +  * when memblock allocator frees pages into buddy system.
>*/
> - zone_init_internals(zone, j, nid, freesize);
> + zone_init_internals(zone, j, nid, 0);

Here, we should initialize zone->managed_pages as zone->present_pages
because later page_group_by_mobility_disabled need be set according to
zone->managed_pages. 

Re: [kvm-unit-tests PATCH v7 06/35] gitlab-ci: Run migration selftest on s390x and powerpc

2024-03-28 Thread Nicholas Piggin
On Tue Mar 26, 2024 at 2:08 AM AEST, Thomas Huth wrote:
> On 19/03/2024 08.58, Nicholas Piggin wrote:
> > The migration harness is complicated and easy to break so CI will
> > be helpful.
> > 
> > Signed-off-by: Nicholas Piggin 
> > ---
> >   .gitlab-ci.yml  | 18 +++---
> >   s390x/unittests.cfg |  8 
> >   2 files changed, 19 insertions(+), 7 deletions(-)
> > 
> > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> > index ff34b1f50..bd34da04f 100644
> > --- a/.gitlab-ci.yml
> > +++ b/.gitlab-ci.yml
> > @@ -92,26 +92,28 @@ build-arm:
> >   build-ppc64be:
> >extends: .outoftree_template
> >script:
> > - - dnf install -y qemu-system-ppc gcc-powerpc64-linux-gnu
> > + - dnf install -y qemu-system-ppc gcc-powerpc64-linux-gnu nmap-ncat
> >- mkdir build
> >- cd build
> >- ../configure --arch=ppc64 --endian=big 
> > --cross-prefix=powerpc64-linux-gnu-
> >- make -j2
> >- ACCEL=tcg ./run_tests.sh
> > - selftest-setup spapr_hcall rtas-get-time-of-day 
> > rtas-get-time-of-day-base
> > - rtas-set-time-of-day emulator
> > + selftest-setup selftest-migration selftest-migration-skip spapr_hcall
> > + rtas-get-time-of-day rtas-get-time-of-day-base rtas-set-time-of-day
>
> I used to squash as much as possible into one line in the past, but nowadays 
> I rather prefer one test per line (like it is done for s390x below), so that 
> it is easier to identify the changes ...
> So if you like, I think you could also put each test on a separate line here 
> now (since you're touching all lines with tests here anyway).

Yeah it is nicer.

>
> > + emulator
> >| tee results.txt
> >- if grep -q FAIL results.txt ; then exit 1 ; fi
> >   
> >   build-ppc64le:
> >extends: .intree_template
> >script:
> > - - dnf install -y qemu-system-ppc gcc-powerpc64-linux-gnu
> > + - dnf install -y qemu-system-ppc gcc-powerpc64-linux-gnu nmap-ncat
> >- ./configure --arch=ppc64 --endian=little 
> > --cross-prefix=powerpc64-linux-gnu-
> >- make -j2
> >- ACCEL=tcg ./run_tests.sh
> > - selftest-setup spapr_hcall rtas-get-time-of-day 
> > rtas-get-time-of-day-base
> > - rtas-set-time-of-day emulator
> > + selftest-setup selftest-migration selftest-migration-skip spapr_hcall
> > + rtas-get-time-of-day rtas-get-time-of-day-base rtas-set-time-of-day
> > + emulator
> >| tee results.txt
> >- if grep -q FAIL results.txt ; then exit 1 ; fi
> >   
> > @@ -135,7 +137,7 @@ build-riscv64:
> >   build-s390x:
> >extends: .outoftree_template
> >script:
> > - - dnf install -y qemu-system-s390x gcc-s390x-linux-gnu
> > + - dnf install -y qemu-system-s390x gcc-s390x-linux-gnu nmap-ncat
> >- mkdir build
> >- cd build
> >- ../configure --arch=s390x --cross-prefix=s390x-linux-gnu-
> > @@ -161,6 +163,8 @@ build-s390x:
> > sclp-1g
> > sclp-3g
> > selftest-setup
> > +  selftest-migration-kvm
> > +  selftest-migration-skip
> > sieve
> > smp
> > stsi
> > diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg
> > index 49e3e4608..b79b99416 100644
> > --- a/s390x/unittests.cfg
> > +++ b/s390x/unittests.cfg
> > @@ -31,6 +31,14 @@ groups = selftest migration
> >   # 
> > https://lore.kernel.org/qemu-devel/20240219061731.232570-1-npig...@gmail.com/
> >   accel = kvm
> >   
> > +[selftest-migration-kvm]
> > +file = selftest-migration.elf
> > +groups = nodefault
> > +accel = kvm
> > +# This is a special test for gitlab-ci that can must not use TCG until the
>
> "can" or "must"?

I think it must be "must not".

Thanks,
Nick


Re: [PATCH v7 6/6] docs: trusted-encrypted: add DCP as new trust source

2024-03-28 Thread David Gstir
Jarkko,

> On 27.03.2024, at 16:40, Jarkko Sakkinen  wrote:
> 
> On Wed Mar 27, 2024 at 10:24 AM EET, David Gstir wrote:
>> Update the documentation for trusted and encrypted KEYS with DCP as new
>> trust source:
>> 
>> - Describe security properties of DCP trust source
>> - Describe key usage
>> - Document blob format
>> 
>> Co-developed-by: Richard Weinberger 
>> Signed-off-by: Richard Weinberger 
>> Co-developed-by: David Oberhollenzer 
>> Signed-off-by: David Oberhollenzer 
>> Signed-off-by: David Gstir 
>> ---
>> .../security/keys/trusted-encrypted.rst   | 85 +++
>> 1 file changed, 85 insertions(+)
>> 
>> diff --git a/Documentation/security/keys/trusted-encrypted.rst 
>> b/Documentation/security/keys/trusted-encrypted.rst
>> index e989b9802f92..81fb3540bb20 100644
>> --- a/Documentation/security/keys/trusted-encrypted.rst
>> +++ b/Documentation/security/keys/trusted-encrypted.rst
>> @@ -42,6 +42,14 @@ safe.
>>  randomly generated and fused into each SoC at manufacturing time.
>>  Otherwise, a common fixed test key is used instead.
>> 
>> + (4) DCP (Data Co-Processor: crypto accelerator of various i.MX SoCs)
>> +
>> + Rooted to a one-time programmable key (OTP) that is generally burnt
>> + in the on-chip fuses and is accessible to the DCP encryption 
>> engine only.
>> + DCP provides two keys that can be used as root of trust: the OTP 
>> key
>> + and the UNIQUE key. Default is to use the UNIQUE key, but selecting
>> + the OTP key can be done via a module parameter (dcp_use_otp_key).
>> +
>>   *  Execution isolation
>> 
>>  (1) TPM
>> @@ -57,6 +65,12 @@ safe.
>> 
>>  Fixed set of operations running in isolated execution environment.
>> 
>> + (4) DCP
>> +
>> + Fixed set of cryptographic operations running in isolated execution
>> + environment. Only basic blob key encryption is executed there.
>> + The actual key sealing/unsealing is done on main processor/kernel 
>> space.
>> +
>>   * Optional binding to platform integrity state
>> 
>>  (1) TPM
>> @@ -79,6 +93,11 @@ safe.
>>  Relies on the High Assurance Boot (HAB) mechanism of NXP SoCs
>>  for platform integrity.
>> 
>> + (4) DCP
>> +
>> + Relies on Secure/Trusted boot process (called HAB by vendor) for
>> + platform integrity.
>> +
>>   *  Interfaces and APIs
>> 
>>  (1) TPM
>> @@ -94,6 +113,11 @@ safe.
>> 
>>  Interface is specific to silicon vendor.
>> 
>> + (4) DCP
>> +
>> + Vendor-specific API that is implemented as part of the DCP crypto 
>> driver in
>> + ``drivers/crypto/mxs-dcp.c``.
>> +
>>   *  Threat model
>> 
>>  The strength and appropriateness of a particular trust source for a 
>> given
>> @@ -129,6 +153,13 @@ selected trust source:
>>  CAAM HWRNG, enable CRYPTO_DEV_FSL_CAAM_RNG_API and ensure the device
>>  is probed.
>> 
>> +  *  DCP (Data Co-Processor: crypto accelerator of various i.MX SoCs)
>> +
>> + The DCP hardware device itself does not provide a dedicated RNG 
>> interface,
>> + so the kernel default RNG is used. SoCs with DCP like the i.MX6ULL do 
>> have
>> + a dedicated hardware RNG that is independent from DCP which can be 
>> enabled
>> + to back the kernel RNG.
>> +
>> Users may override this by specifying ``trusted.rng=kernel`` on the kernel
>> command-line to override the used RNG with the kernel's random number pool.
>> 
>> @@ -231,6 +262,19 @@ Usage::
>> CAAM-specific format.  The key length for new keys is always in bytes.
>> Trusted Keys can be 32 - 128 bytes (256 - 1024 bits).
>> 
>> +Trusted Keys usage: DCP
>> +---
>> +
>> +Usage::
>> +
>> +keyctl add trusted name "new keylen" ring
>> +keyctl add trusted name "load hex_blob" ring
>> +keyctl print keyid
>> +
>> +"keyctl print" returns an ASCII hex copy of the sealed key, which is in 
>> format
>> +specific to this DCP key-blob implementation.  The key length for new keys 
>> is
>> +always in bytes. Trusted Keys can be 32 - 128 bytes (256 - 1024 bits).
>> +
>> Encrypted Keys usage
>> 
>> 
>> @@ -426,3 +470,44 @@ string length.
>> privkey is the binary representation of TPM2B_PUBLIC excluding the
>> initial TPM2B header which can be reconstructed from the ASN.1 octed
>> string length.
>> +
>> +DCP Blob Format
>> +---
>> +
>> +The Data Co-Processor (DCP) provides hardware-bound AES keys using its
>> +AES encryption engine only. It does not provide direct key 
>> sealing/unsealing.
>> +To make DCP hardware encryption keys usable as trust source, we define
>> +our own custom format that uses a hardware-bound key to secure the sealing
>> +key stored in the key blob.
>> +
>> +Whenever a new trusted key using DCP is generated, we generate a random 
>> 128-bit
>> +blob encryption key (BEK) and 128-bit nonce. The BEK and nonce are used to
>> +encrypt the trusted key payload using AES-128-GCM.
>> 

Re: [PATCH v11 00/11] Support page table check PowerPC

2024-03-28 Thread Christophe Leroy


Le 28/03/2024 à 07:52, Christophe Leroy a écrit :
> 
> 
> Le 28/03/2024 à 05:55, Rohan McLure a écrit :
>> Support page table check on all PowerPC platforms. This works by
>> serialising assignments, reassignments and clears of page table
>> entries at each level in order to ensure that anonymous mappings
>> have at most one writable consumer, and likewise that file-backed
>> mappings are not simultaneously also anonymous mappings.
>>
>> In order to support this infrastructure, a number of stubs must be
>> defined for all powerpc platforms. Additionally, seperate set_pte_at()
>> and set_pte_at_unchecked(), to allow for internal, uninstrumented 
>> mappings.
> 
> I gave it a try on QEMU e500 (64 bits), and get the following Oops. What 
> do I have to look for ?
> 
> Freeing unused kernel image (initmem) memory: 2588K
> This architecture does not have kernel memory protection.
> Run /init as init process
> [ cut here ]
> kernel BUG at mm/page_table_check.c:119!
> Oops: Exception in kernel mode, sig: 5 [#1]
> BE PAGE_SIZE=4K SMP NR_CPUS=32 QEMU e500

Same problem on my 8xx board:

[7.358146] Freeing unused kernel image (initmem) memory: 448K
[7.363957] Run /init as init process
[7.370955] [ cut here ]
[7.375411] kernel BUG at mm/page_table_check.c:119!
[7.380393] Oops: Exception in kernel mode, sig: 5 [#1]
[7.385621] BE PAGE_SIZE=16K PREEMPT CMPC885
[7.393483] CPU: 0 PID: 1 Comm: init Not tainted 
6.8.0-s3k-dev-13737-g8d9e247585fb #787
[7.401505] Hardware name: MIAE 8xx 0x50 CMPC885
[7.406481] NIP:  c0183278 LR: c018316c CTR: 0001
[7.411541] REGS: c902bb20 TRAP: 0700   Not tainted 
(6.8.0-s3k-dev-13737-g8d9e247585fb)
[7.419657] MSR:  00029032   CR: 35055355  XER: 80007100
[7.426550]
[7.426550] GPR00: c018316c c902bbe0 c2118000 c7f7a0d8 7fab8000 
c23b5ae0 c902bc20 0002
[7.426550] GPR08: c11a c7f7a0d8 c11143e0  95003355 
 c0004a38 c23a0a00
[7.426550] GPR16: 4000 7fffc000 8000 c23a0a00 0001 
7fab8000 ffabc000 8000
[7.426550] GPR24: 7fffc000 c33be1c0 4000 c902bc20 7fab8000 
0001 c7fb0360 
[7.463291] NIP [c0183278] __page_table_check_ptes_set+0x1c8/0x210
[7.469491] LR [c018316c] __page_table_check_ptes_set+0xbc/0x210
[7.475514] Call Trace:
[7.477957] [c902bbe0] [c018316c] 
__page_table_check_ptes_set+0xbc/0x210 (unreliable)
[7.485809] [c902bc00] [c0012474] set_ptes+0x148/0x16c
[7.490958] [c902bc50] [c0158a3c] move_page_tables+0x228/0x578
[7.496806] [c902bcf0] [c0192f38] shift_arg_pages+0xf0/0x1d4
[7.502479] [c902bd90] [c0193b40] setup_arg_pages+0x1c8/0x36c
[7.508238] [c902be40] [c01f51a0] load_elf_binary+0x3c0/0x1218
[7.514086] [c902beb0] [c01934b0] bprm_execve+0x21c/0x4a4
[7.519497] [c902bf00] [c019516c] kernel_execve+0x13c/0x200
[7.525082] [c902bf20] [c0004aa8] kernel_init+0x70/0x1b0
[7.530406] [c902bf30] [c00111e4] ret_from_kernel_user_thread+0x10/0x18
[7.537038] --- interrupt: 0 at 0x0
[7.540534] Code: 39290004 7ce04828 30e70001 7ce0492d 40a2fff4 
2c07 4080ff94 0fe0 0fe0 0fe0 2c1f 4082ff80 
<0fe0> 0fe0 392a 4bfffef8
[7.556068] ---[ end trace  ]---
[7.560692]
[8.531997] note: init[1] exited with irqs disabled
[8.536891] note: init[1] exited with preempt_count 1
[8.542032] Kernel panic - not syncing: Attempted to kill init! 
exitcode=0x0005
[8.549602] Rebooting in 180 seconds..


Re: [PATCH v2 2/3] arch: Remove struct fb_info from video helpers

2024-03-28 Thread Sam Ravnborg
Hi Thomas,
On Wed, Mar 27, 2024 at 09:41:30PM +0100, Thomas Zimmermann wrote:
> The per-architecture video helpers do not depend on struct fb_info
> or anything else from fbdev. Remove it from the interface and replace
> fb_is_primary_device() with video_is_primary_device(). The new helper
> is similar in functionality, but can operate on non-fbdev devices.
> 
> Signed-off-by: Thomas Zimmermann 
> Cc: "James E.J. Bottomley" 
> Cc: Helge Deller 
> Cc: "David S. Miller" 
> Cc: Andreas Larsson 
> Cc: Thomas Gleixner 
> Cc: Ingo Molnar 
> Cc: Borislav Petkov 
> Cc: Dave Hansen 
> Cc: x...@kernel.org
> Cc: "H. Peter Anvin" 
Reviewed-by: Sam Ravnborg 


Re: [PATCH RFC 0/3] mm/gup: consistently call it GUP-fast

2024-03-28 Thread Mike Rapoport
On Thu, Mar 28, 2024 at 07:09:13AM +0100, Arnd Bergmann wrote:
> On Thu, Mar 28, 2024, at 06:51, Vineet Gupta wrote:
> > On 3/27/24 09:22, Arnd Bergmann wrote:
> >> On Wed, Mar 27, 2024, at 16:39, David Hildenbrand wrote:
> >>> On 27.03.24 16:21, Peter Xu wrote:
>  On Wed, Mar 27, 2024 at 02:05:35PM +0100, David Hildenbrand wrote:
> 
>  I'm not sure what config you tried there; as I am doing some build tests
>  recently, I found turning off CONFIG_SAMPLES + CONFIG_GCC_PLUGINS could
>  avoid a lot of issues, I think it's due to libc missing.  But maybe not 
>  the
>  case there.
> >>> CCin Arnd; I use some of his compiler chains, others from Fedora 
> >>> directly. For
> >>> example for alpha and arc, the Fedora gcc is "13.2.1".
> >>> But there is other stuff like (arc):
> >>>
> >>> ./arch/arc/include/asm/mmu-arcv2.h: In function 'mmu_setup_asid':
> >>> ./arch/arc/include/asm/mmu-arcv2.h:82:9: error: implicit declaration of 
> >>> function 'write_aux_reg' [-Werro
> >>> r=implicit-function-declaration]
> >>> 82 | write_aux_reg(ARC_REG_PID, asid | MMU_ENABLE);
> >>>| ^
> >> Seems to be missing an #include of soc/arc/aux.h, but I can't
> >> tell when this first broke without bisecting.
> >
> > Weird I don't see this one but I only have gcc 12 handy ATM.
> >
> >     gcc version 12.2.1 20230306 (ARC HS GNU/Linux glibc toolchain -
> > build 1360)
> >
> > I even tried W=1 (which according to scripts/Makefile.extrawarn) should
> > include -Werror=implicit-function-declaration but don't see this still.
> >
> > Tomorrow I'll try building a gcc 13.2.1 for ARC.
> 
> David reported them with the toolchains I built at
> https://mirrors.edge.kernel.org/pub/tools/crosstool/
> I'm fairly sure the problem is specific to the .config
> and tree, not the toolchain though.

This happens with defconfig and both gcc 12.2.0 and gcc 13.2.0 from your
crosstools. I also see these on the current Linus' tree:

arc/kernel/ptrace.c:342:16: warning: no previous prototype for 
'syscall_trace_enter' [-Wmissing-prototypes]
arch/arc/kernel/kprobes.c:193:15: warning: no previous prototype for 
'arc_kprobe_handler' [-Wmissing-prototypes]

This fixed the warning about write_aux_reg for me, probably Vineet would
want this include somewhere else...

diff --git a/arch/arc/include/asm/mmu-arcv2.h b/arch/arc/include/asm/mmu-arcv2.h
index ed9036d4ede3..0fca342d7b79 100644
--- a/arch/arc/include/asm/mmu-arcv2.h
+++ b/arch/arc/include/asm/mmu-arcv2.h
@@ -69,6 +69,8 @@
 
 #ifndef __ASSEMBLY__
 
+#include 
+
 struct mm_struct;
 extern int pae40_exist_but_not_enab(void);
 
 
> >>> or (alpha)
> >>>
> >>> WARNING: modpost: "saved_config" [vmlinux] is COMMON symbol
> >>> ERROR: modpost: "memcpy" [fs/reiserfs/reiserfs.ko] undefined!
> >>> ERROR: modpost: "memcpy" [fs/nfs/nfs.ko] undefined!
> >>> ERROR: modpost: "memcpy" [fs/nfs/nfsv3.ko] undefined!
> >>> ERROR: modpost: "memcpy" [fs/nfsd/nfsd.ko] undefined!
> >>> ERROR: modpost: "memcpy" [fs/lockd/lockd.ko] undefined!
> >>> ERROR: modpost: "memcpy" [crypto/crypto.ko] undefined!
> >>> ERROR: modpost: "memcpy" [crypto/crypto_algapi.ko] undefined!
> >>> ERROR: modpost: "memcpy" [crypto/aead.ko] undefined!
> >>> ERROR: modpost: "memcpy" [crypto/crypto_skcipher.ko] undefined!
> >>> ERROR: modpost: "memcpy" [crypto/seqiv.ko] undefined!
> >
> > Are these from ARC build or otherwise ?
> 
> This was arch/alpha.
> 
>   Arnd

-- 
Sincerely yours,
Mike.


Re: [PATCH v11 00/11] Support page table check PowerPC

2024-03-28 Thread Christophe Leroy


Le 28/03/2024 à 05:55, Rohan McLure a écrit :
> Support page table check on all PowerPC platforms. This works by
> serialising assignments, reassignments and clears of page table
> entries at each level in order to ensure that anonymous mappings
> have at most one writable consumer, and likewise that file-backed
> mappings are not simultaneously also anonymous mappings.
> 
> In order to support this infrastructure, a number of stubs must be
> defined for all powerpc platforms. Additionally, seperate set_pte_at()
> and set_pte_at_unchecked(), to allow for internal, uninstrumented mappings.

I gave it a try on QEMU e500 (64 bits), and get the following Oops. What 
do I have to look for ?

Freeing unused kernel image (initmem) memory: 2588K
This architecture does not have kernel memory protection.
Run /init as init process
[ cut here ]
kernel BUG at mm/page_table_check.c:119!
Oops: Exception in kernel mode, sig: 5 [#1]
BE PAGE_SIZE=4K SMP NR_CPUS=32 QEMU e500
Modules linked in:
CPU: 0 PID: 1 Comm: init Not tainted 6.8.0-13732-gc5347beead0b-dirty #784
Hardware name: QEMU ppce500 e5500 0x80240020 QEMU e500
NIP:  c02951a0 LR: c02951bc CTR: 
REGS: c32e7440 TRAP: 0700   Not tainted 
(6.8.0-13732-gc5347beead0b-dirty)
MSR:  80029002   CR: 24044248  XER: 
IRQMASK: 0
GPR00: c0029d90 c32e76e0 c0d44000 c3017e18
GPR04: ffb11000 c7f16888 000fc324123d 
GPR08:  0001 c1184000 84004248
GPR12: 00c0 c11b9000 c7f16888 c7f19000
GPR16: 1000 3000  
GPR20: 4000  0001 c000ffb12000
GPR24: c7f19000 c6008000 c6008000 0030
GPR28: 0001 c118afe8 c3017e18 0001
NIP [c02951a0] __page_table_check_ptes_set+0x210/0x2ac
LR [c02951bc] __page_table_check_ptes_set+0x22c/0x2ac
Call Trace:
[c32e76e0] [c32e7790] 0xc32e7790 (unreliable)
[c32e7730] [c0029d90] set_ptes+0x178/0x210
[c32e7790] [c024c72c] move_page_tables+0x298/0x750
[c32e7870] [c02a944c] shift_arg_pages+0x120/0x254
[c32e79a0] [c02a9f94] setup_arg_pages+0x244/0x418
[c32e7b30] [c0331610] load_elf_binary+0x584/0x17d4
[c32e7c30] [c02aa3e8] bprm_execve+0x280/0x704
[c32e7d00] [c02ac158] kernel_execve+0x16c/0x214
[c32e7d50] [c00011c8] run_init_process+0x100/0x168
[c32e7de0] [c000214c] kernel_init+0x84/0x1f8
[c32e7e50] [c594] ret_from_kernel_user_thread+0x14/0x1c
--- interrupt: 0 at 0x0
Code: 81230004 7d2907b4 0b09 7c0004ac 7d201828 31290001 7d20192d 
40c2fff4 7c0004ac 2c090002 3920 7d29e01e <0b09> e93d 
37ff 7fde4a14
---[ end trace  ]---

note: init[1] exited with irqs disabled
Kernel panic - not syncing: Attempted to kill init! exitcode=0x0005
Rebooting in 180 seconds..


Re: [PATCH RFC 0/3] mm/gup: consistently call it GUP-fast

2024-03-28 Thread Arnd Bergmann
On Thu, Mar 28, 2024, at 06:51, Vineet Gupta wrote:
> On 3/27/24 09:22, Arnd Bergmann wrote:
>> On Wed, Mar 27, 2024, at 16:39, David Hildenbrand wrote:
>>> On 27.03.24 16:21, Peter Xu wrote:
 On Wed, Mar 27, 2024 at 02:05:35PM +0100, David Hildenbrand wrote:

 I'm not sure what config you tried there; as I am doing some build tests
 recently, I found turning off CONFIG_SAMPLES + CONFIG_GCC_PLUGINS could
 avoid a lot of issues, I think it's due to libc missing.  But maybe not the
 case there.
>>> CCin Arnd; I use some of his compiler chains, others from Fedora directly. 
>>> For
>>> example for alpha and arc, the Fedora gcc is "13.2.1".
>>> But there is other stuff like (arc):
>>>
>>> ./arch/arc/include/asm/mmu-arcv2.h: In function 'mmu_setup_asid':
>>> ./arch/arc/include/asm/mmu-arcv2.h:82:9: error: implicit declaration of 
>>> function 'write_aux_reg' [-Werro
>>> r=implicit-function-declaration]
>>> 82 | write_aux_reg(ARC_REG_PID, asid | MMU_ENABLE);
>>>| ^
>> Seems to be missing an #include of soc/arc/aux.h, but I can't
>> tell when this first broke without bisecting.
>
> Weird I don't see this one but I only have gcc 12 handy ATM.
>
>     gcc version 12.2.1 20230306 (ARC HS GNU/Linux glibc toolchain -
> build 1360)
>
> I even tried W=1 (which according to scripts/Makefile.extrawarn) should
> include -Werror=implicit-function-declaration but don't see this still.
>
> Tomorrow I'll try building a gcc 13.2.1 for ARC.

David reported them with the toolchains I built at
https://mirrors.edge.kernel.org/pub/tools/crosstool/
I'm fairly sure the problem is specific to the .config
and tree, not the toolchain though.

>>> or (alpha)
>>>
>>> WARNING: modpost: "saved_config" [vmlinux] is COMMON symbol
>>> ERROR: modpost: "memcpy" [fs/reiserfs/reiserfs.ko] undefined!
>>> ERROR: modpost: "memcpy" [fs/nfs/nfs.ko] undefined!
>>> ERROR: modpost: "memcpy" [fs/nfs/nfsv3.ko] undefined!
>>> ERROR: modpost: "memcpy" [fs/nfsd/nfsd.ko] undefined!
>>> ERROR: modpost: "memcpy" [fs/lockd/lockd.ko] undefined!
>>> ERROR: modpost: "memcpy" [crypto/crypto.ko] undefined!
>>> ERROR: modpost: "memcpy" [crypto/crypto_algapi.ko] undefined!
>>> ERROR: modpost: "memcpy" [crypto/aead.ko] undefined!
>>> ERROR: modpost: "memcpy" [crypto/crypto_skcipher.ko] undefined!
>>> ERROR: modpost: "memcpy" [crypto/seqiv.ko] undefined!
>
> Are these from ARC build or otherwise ?

This was arch/alpha.

  Arnd