Re: [PATCH 5.10] x86/xen: Drop USERGS_SYSRET64 paravirt call

[Greg Kroah-Hartman]

On Wed, May 22, 2024 at 06:20:15PM -0700, Pawan Gupta wrote:
> From: Juergen Gross 
> 
> commit afd30525a659ac0ae0904f0cb4a2ca75522c3123 upstream.

Now queued up, thanks.

greg k-h

Re: [PATCH 5.4] xen/events: close evtchn after mapping cleanup

[Greg Kroah-Hartman]

On Sat, Mar 02, 2024 at 08:45:39PM -0800, Andrew Paniakin wrote:
> From: Andrew Panyakin 
> 
> From: Maximilian Heyne 
> 
> Commit fa765c4b4aed2d64266b694520ecb025c862c5a9 upstream
> 
> shutdown_pirq and startup_pirq are not taking the
> irq_mapping_update_lock because they can't due to lock inversion. Both
> are called with the irq_desc->lock being taking. The lock order,
> however, is first irq_mapping_update_lock and then irq_desc->lock.
> 
> This opens multiple races:
> - shutdown_pirq can be interrupted by a function that allocates an event
>   channel:
> 
>   CPU0CPU1
>   shutdown_pirq {
> xen_evtchn_close(e)
>   __startup_pirq {
> EVTCHNOP_bind_pirq
>   -> returns just freed evtchn e
> set_evtchn_to_irq(e, irq)
>   }
> xen_irq_info_cleanup() {
>   set_evtchn_to_irq(e, -1)
> }
>   }
> 
>   Assume here event channel e refers here to the same event channel
>   number.
>   After this race the evtchn_to_irq mapping for e is invalid (-1).
> 
> - __startup_pirq races with __unbind_from_irq in a similar way. Because
>   __startup_pirq doesn't take irq_mapping_update_lock it can grab the
>   evtchn that __unbind_from_irq is currently freeing and cleaning up. In
>   this case even though the event channel is allocated, its mapping can
>   be unset in evtchn_to_irq.
> 
> The fix is to first cleanup the mappings and then close the event
> channel. In this way, when an event channel gets allocated it's
> potential previous evtchn_to_irq mappings are guaranteed to be unset already.
> This is also the reverse order of the allocation where first the event
> channel is allocated and then the mappings are setup.
> 
> On a 5.10 kernel prior to commit 3fcdaf3d7634 ("xen/events: modify internal
> [un]bind interfaces"), we hit a BUG like the following during probing of NVMe
> devices. The issue is that during nvme_setup_io_queues, pci_free_irq
> is called for every device which results in a call to shutdown_pirq.
> With many nvme devices it's therefore likely to hit this race during
> boot because there will be multiple calls to shutdown_pirq and
> startup_pirq are running potentially in parallel.
> 
>   [ cut here ]
>   blkfront: xvda: barrier or flush: disabled; persistent grants: enabled; 
> indirect descriptors: enabled; bounce buffer: enabled
>   kernel BUG at drivers/xen/events/events_base.c:499!
>   invalid opcode:  [#1] SMP PTI
>   CPU: 44 PID: 375 Comm: kworker/u257:23 Not tainted 
> 5.10.201-191.748.amzn2.x86_64 #1
>   Hardware name: Xen HVM domU, BIOS 4.11.amazon 08/24/2006
>   Workqueue: nvme-reset-wq nvme_reset_work
>   RIP: 0010:bind_evtchn_to_cpu+0xdf/0xf0
>   Code: 5d 41 5e c3 cc cc cc cc 44 89 f7 e8 2b 55 ad ff 49 89 c5 48 85 c0 0f 
> 84 64 ff ff ff 4c 8b 68 30 41 83 fe ff 0f 85 60 ff ff ff <0f> 0b 66 66 2e 0f 
> 1f 84 00 00 00 00 00 0f 1f 40 00 0f 1f 44 00 00
>   RSP: :c9000d533b08 EFLAGS: 00010046
>   RAX:  RBX:  RCX: 0006
>   RDX: 0028 RSI:  RDI: 
>   RBP: 888107419680 R08:  R09: 82d72b00
>   R10:  R11:  R12: 01ed
>   R13:  R14:  R15: 0002
>   FS:  () GS:88bc8b50() knlGS:
>   CS:  0010 DS:  ES:  CR0: 80050033
>   CR2:  CR3: 02610001 CR4: 001706e0
>   DR0:  DR1:  DR2: 
>   DR3:  DR6: fffe0ff0 DR7: 0400
>   Call Trace:
>? show_trace_log_lvl+0x1c1/0x2d9
>? show_trace_log_lvl+0x1c1/0x2d9
>? set_affinity_irq+0xdc/0x1c0
>? __die_body.cold+0x8/0xd
>? die+0x2b/0x50
>? do_trap+0x90/0x110
>? bind_evtchn_to_cpu+0xdf/0xf0
>? do_error_trap+0x65/0x80
>? bind_evtchn_to_cpu+0xdf/0xf0
>? exc_invalid_op+0x4e/0x70
>? bind_evtchn_to_cpu+0xdf/0xf0
>? asm_exc_invalid_op+0x12/0x20
>? bind_evtchn_to_cpu+0xdf/0xf0
>? bind_evtchn_to_cpu+0xc5/0xf0
>set_affinity_irq+0xdc/0x1c0
>irq_do_set_affinity+0x1d7/0x1f0
>irq_setup_affinity+0xd6/0x1a0
>irq_startup+0x8a/0xf0
>__setup_irq+0x639/0x6d0
>? nvme_suspend+0x150/0x150
>request_threaded_irq+0x10c/0x180
>? nvme_suspend+0x150/0x150
>pci_request_irq+0xa8/0xf0
>? __blk_mq_free_request+0x74/0xa0
>queue_request_irq+0x6f/0x80
>nvme_create_queue+0x1af/0x200
>nvme_create_io_queues+0xbd/0xf0
>nvme_setup_io_queues+0x246/0x320
>? nvme_irq_check+0x30/0x30
>nvme_reset_work+0x1c8/0x400
>process_one_work+0x1b0/0x350
>worker_thread+0x49/0x310
>? process_one_work+0x350/0x350
>kthread+0x11b/0x140
>? __kthread_bind_mask+0x60/0x60
>ret_from_fork+0x22/0x30
>   Mod

Re: [PATCH 0/2] drivers: xen: struct bus_type cleanup

[Greg Kroah-Hartman]

On Sat, Feb 03, 2024 at 03:53:36PM -0300, Ricardo B. Marliere wrote:
> This series is part of an effort to cleanup the users of the driver
> core, as can be seen in many recent patches authored by Greg across the
> tree (e.g. [1]). Specifically, this series is part of the task of
> splitting one of his TODOs [2].
> 
> ---
> [1]: 
> https://lore.kernel.org/lkml/?q=f%3Agregkh%40linuxfoundation.org+s%3A%22make%22+and+s%3A%22const%22
> [2]: 
> https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git/commit/?h=bus_cleanup&id=26105f537f0c60eacfeb430abd2e05d7ddcdd8aa
> 
> Cc: Greg Kroah-Hartman 
> Signed-off-by: Ricardo B. Marliere 

Reviewed-by: Greg Kroah-Hartman 

Re: [PATCH v2 1/3] hvc/xen: fix event channel handling for secondary consoles

[Greg Kroah-Hartman]

On Fri, Oct 20, 2023 at 05:15:27PM +0100, David Woodhouse wrote:
> From: David Woodhouse 
> 
> The xencons_connect_backend() function allocates a local interdomain
> event channel with xenbus_alloc_evtchn(), then calls
> bind_interdomain_evtchn_to_irq_lateeoi() to bind to that port# on the
> *remote* domain.
> 
> That doesn't work very well:
> 
> (qemu) device_add xen-console,id=con1,chardev=pty0
> [   44.323872] xenconsole console-1: 2 xenbus_dev_probe on device/console/1
> [   44.323995] xenconsole: probe of console-1 failed with error -2
> 
> Fix it to use bind_evtchn_to_irq_lateeoi(), which does the right thing
> by just binding that *local* event channel to an irq. The backend will
> do the interdomain binding.
> 
> This didn't affect the primary console because the setup for that is
> special — the toolstack allocates the guest event channel and the guest
> discovers it with HVMOP_get_param.
> 
> Fixes: fe415186b4 ("xen/console: harden hvc_xen against event channel storms")

Nit, our tools complain that the sha1 isn't big enough, "fe415186b43d"
I'll go fix it up...

Re: [PATCH 04/15] tty: Remove now superfluous sentinel element from ctl_table array

[Greg Kroah-Hartman]

On Mon, Oct 02, 2023 at 08:47:53AM +, Christophe Leroy wrote:
> 
> 
> Le 02/10/2023 à 10:17, Jiri Slaby a écrit :
> > On 28. 09. 23, 15:21, Joel Granados via B4 Relay wrote:
> >> From: Joel Granados 
> >>
> >> This commit comes at the tail end of a greater effort to remove the
> >> empty elements at the end of the ctl_table arrays (sentinels) which
> >> will reduce the overall build time size of the kernel and run time
> >> memory bloat by ~64 bytes per sentinel (further information Link :
> >> https://lore.kernel.org/all/zo5yx5jfoggi%2f...@bombadil.infradead.org/)
> >>
> >> Remove sentinel from tty_table
> >>
> >> Signed-off-by: Joel Granados 
> >> ---
> >>   drivers/tty/tty_io.c | 3 +--
> >>   1 file changed, 1 insertion(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
> >> index 8a94e5a43c6d..2f925dc54a20 100644
> >> --- a/drivers/tty/tty_io.c
> >> +++ b/drivers/tty/tty_io.c
> >> @@ -3607,8 +3607,7 @@ static struct ctl_table tty_table[] = {
> >>   .proc_handler    = proc_dointvec,
> >>   .extra1    = SYSCTL_ZERO,
> >>   .extra2    = SYSCTL_ONE,
> >> -    },
> >> -    { }
> >> +    }
> > 
> > Why to remove the comma? One would need to add one when adding a new entry?
> 
> Does it make any difference at all ?
> 
> In one case you have:
> 
> @
>   something old,
>   },
> + {
> + something new,
> + },
>   }
> 
> In the other case you have:
> 
> @
>   something old,
> + },
> + {
> + something new,
>   }
>   }

Because that way it is obvious you are only touching the "something new"
lines and never have to touch the "something old" ones.

It's just a long-standing tradition in Linux, don't have an extra
character if you don't need it :)

thanks,

greg k-h

Re: [PATCH 01/15] cdrom: Remove now superfluous sentinel element from ctl_table array

[Greg Kroah-Hartman]

On Thu, Sep 28, 2023 at 03:21:26PM +0200, Joel Granados via B4 Relay wrote:
> From: Joel Granados 
> 
> This commit comes at the tail end of a greater effort to remove the
> empty elements at the end of the ctl_table arrays (sentinels) which
> will reduce the overall build time size of the kernel and run time
> memory bloat by ~64 bytes per sentinel (further information Link :
> https://lore.kernel.org/all/zo5yx5jfoggi%2f...@bombadil.infradead.org/)
> 
> Remove sentinel element from cdrom_table
> 
> Signed-off-by: Joel Granados 
> ---
>  drivers/cdrom/cdrom.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c
> index cc2839805983..451907ade389 100644
> --- a/drivers/cdrom/cdrom.c
> +++ b/drivers/cdrom/cdrom.c
> @@ -3654,8 +3654,7 @@ static struct ctl_table cdrom_table[] = {
>   .maxlen = sizeof(int),
>   .mode   = 0644,
>   .proc_handler   = cdrom_sysctl_handler
> - },
> - { }
> + }

You should have the final entry as "}," so as to make any future
additions to the list to only contain that entry, that's long been the
kernel style for lists like this.

So your patches will just remove one line, not 2 and add 1, making it a
smaller diff.

thanks,

greg k-h

Re: [PATCH v7 0/9] Allow dynamic allocation of software IO TLB bounce buffers

[Greg Kroah-Hartman]

nel was booted with "swiotlb=force" to allow stress-testing
> the software IO TLB on a high-performance device that would otherwise not
> need it. CONFIG_DEBUG_FS was set to 'y' to match the configuration of
> popular distribution kernels; it is understood that parallel workloads
> suffer from contention on the recently added debugfs atomic counters.
> 
> These benchmarks were run:
> 
> - small: single-threaded I/O of 4 KiB blocks,
> - big: single-threaded I/O of 64 KiB blocks,
> - 4way: 4-way parallel I/O of 4 KiB blocks.
> 
> In all tested cases, the default 64 MiB SWIOTLB would be sufficient (but
> wasteful). The "default" pair of columns shows performance impact when
> booted with 64 MiB SWIOTLB (i.e. current state). The "growing" pair of
> columns shows the impact when booted with a 1 MiB initial SWIOTLB, which
> grew to 5 MiB at run time. The "var" column in the tables below is the
> coefficient of variance over 5 runs of the test, the "diff" column is the
> difference in read-write I/O bandwidth (MiB/s). The very first column is
> the coefficient of variance in the results of the base unpatched kernel.
> 
> First, on an x86 VM against a QEMU virtio SATA driver backed by a RAM-based
> block device on the host:
> 
>   base   default growing
>   var var diffvar diff
> small 1.96%   0.47%   -1.5%   0.52%   -2.2%
> big   2.03%   1.35%   +0.9%   2.22%   +2.9%
> 4way  0.80%   0.45%   -0.7%   1.22%   <0.1%
> 
> Second, on a Raspberry Pi4 with 8G RAM and a class 10 A1 microSD card:
> 
>   base   default growing
>   var var diffvar diff
> small 1.09%   1.69%   +0.5%   2.14%   -0.2%
> big   0.03%   0.28%   -0.5%   0.03%   -0.1%
> 4way  5.15%   2.39%   +0.2%   0.66%   <0.1%
> 
> Third, on a CoCo VM. This was a bigger system, so I also added a 24-thread
> parallel I/O test:
> 
>   base   default growing
>   var var diffvar diff
> small 2.41%   6.02%   +1.1%   10.33%  +6.7%
> big   9.20%   2.81%   -0.6%   16.84%  -0.2%
> 4way  0.86%   2.66%   -0.1%2.22%  -4.9%
> 24way 3.19%   6.19%   +4.4%4.08%  -5.9%
> 
> Note the increased variance of the CoCo VM, although the host was not
> otherwise loaded. These are caused by the first run, which includes the
> overhead of allocating additional bounce buffers and sharing them with the
> hypervisor. The system was not rebooted between successive runs.
> 
> Parallel tests suffer from a reduced number of areas in the dynamically
> allocated memory pools. This can be improved by allocating a larger pool
> from CMA (not implemented in this series yet).
> 
> I have no good explanation for the increase in performance of the
> 24-thread I/O test with the default (non-growing) memory pool. Although the
> difference is within variance, it seems to be real. The average bandwidth
> is consistently above that of the unpatched kernel.
> 
> To sum it up:
> 
> - All workloads benefit from reduced memory footprint.
> - No performance regressions have been observed with the default size of
>   the software IO TLB.
> - Most workloads retain their former performance even if the software IO
>   TLB grows at run time.
> 

For the driver-core touched portions:

Reviewed-by: Greg Kroah-Hartman 

Re: [PATCH v3 4/7] swiotlb: if swiotlb is full, fall back to a transient memory pool

[Greg Kroah-Hartman]

On Thu, Jul 06, 2023 at 02:22:50PM +, Michael Kelley (LINUX) wrote:
> From: Greg Kroah-Hartman  Sent: Thursday, July 6, 
> 2023 1:07 AM
> > 
> > On Thu, Jul 06, 2023 at 03:50:55AM +, Michael Kelley (LINUX) wrote:
> > > From: Petr Tesarik  Sent: Tuesday, June 27, 
> > > 2023
> > 2:54 AM
> > > >
> > > > Try to allocate a transient memory pool if no suitable slots can be 
> > > > found,
> > > > except when allocating from a restricted pool. The transient pool is 
> > > > just
> > > > enough big for this one bounce buffer. It is inserted into a per-device
> > > > list of transient memory pools, and it is freed again when the bounce
> > > > buffer is unmapped.
> > > >
> > > > Transient memory pools are kept in an RCU list. A memory barrier is
> > > > required after adding a new entry, because any address within a 
> > > > transient
> > > > buffer must be immediately recognized as belonging to the SWIOTLB, even 
> > > > if
> > > > it is passed to another CPU.
> > > >
> > > > Deletion does not require any synchronization beyond RCU ordering
> > > > guarantees. After a buffer is unmapped, its physical addresses may no
> > > > longer be passed to the DMA API, so the memory range of the 
> > > > corresponding
> > > > stale entry in the RCU list never matches. If the memory range gets
> > > > allocated again, then it happens only after a RCU quiescent state.
> > > >
> > > > Since bounce buffers can now be allocated from different pools, add a
> > > > parameter to swiotlb_alloc_pool() to let the caller know which memory 
> > > > pool
> > > > is used. Add swiotlb_find_pool() to find the memory pool corresponding 
> > > > to
> > > > an address. This function is now also used by is_swiotlb_buffer(), 
> > > > because
> > > > a simple boundary check is no longer sufficient.
> > > >
> > > > The logic in swiotlb_alloc_tlb() is taken from 
> > > > __dma_direct_alloc_pages(),
> > > > simplified and enhanced to use coherent memory pools if needed.
> > > >
> > > > Note that this is not the most efficient way to provide a bounce buffer,
> > > > but when a DMA buffer can't be mapped, something may (and will) actually
> > > > break. At that point it is better to make an allocation, even if it may 
> > > > be
> > > > an expensive operation.
> > >
> > > I continue to think about swiotlb memory management from the standpoint
> > > of CoCo VMs that may be quite large with high network and storage loads.
> > > These VMs are often running mission-critical workloads that can't tolerate
> > > a bounce buffer allocation failure.  To prevent such failures, the swiotlb
> > > memory size must be overly large, which wastes memory.
> > 
> > If "mission critical workloads" are in a vm that allowes overcommit and
> > no control over other vms in that same system, then you have worse
> > problems, sorry.
> > 
> > Just don't do that.
> > 
> 
> No, the cases I'm concerned about don't involve memory overcommit.
> 
> CoCo VMs must use swiotlb bounce buffers to do DMA I/O.  Current swiotlb
> code in the Linux guest allocates a configurable, but fixed, amount of guest
> memory at boot time for this purpose.  But it's hard to know how much
> swiotlb bounce buffer memory will be needed to handle peak I/O loads.
> This patch set does dynamic allocation of swiotlb bounce buffer memory,
> which can help avoid needing to configure an overly large fixed size at boot.

But, as you point out, memory allocation can fail at runtime, so how can
you "guarantee" that this will work properly anymore if you are going to
make it dynamic?

confused,

greg k-h

Re: [PATCH v3 4/7] swiotlb: if swiotlb is full, fall back to a transient memory pool

[Greg Kroah-Hartman]

On Thu, Jul 06, 2023 at 03:50:55AM +, Michael Kelley (LINUX) wrote:
> From: Petr Tesarik  Sent: Tuesday, June 27, 2023 
> 2:54 AM
> > 
> > Try to allocate a transient memory pool if no suitable slots can be found,
> > except when allocating from a restricted pool. The transient pool is just
> > enough big for this one bounce buffer. It is inserted into a per-device
> > list of transient memory pools, and it is freed again when the bounce
> > buffer is unmapped.
> > 
> > Transient memory pools are kept in an RCU list. A memory barrier is
> > required after adding a new entry, because any address within a transient
> > buffer must be immediately recognized as belonging to the SWIOTLB, even if
> > it is passed to another CPU.
> > 
> > Deletion does not require any synchronization beyond RCU ordering
> > guarantees. After a buffer is unmapped, its physical addresses may no
> > longer be passed to the DMA API, so the memory range of the corresponding
> > stale entry in the RCU list never matches. If the memory range gets
> > allocated again, then it happens only after a RCU quiescent state.
> > 
> > Since bounce buffers can now be allocated from different pools, add a
> > parameter to swiotlb_alloc_pool() to let the caller know which memory pool
> > is used. Add swiotlb_find_pool() to find the memory pool corresponding to
> > an address. This function is now also used by is_swiotlb_buffer(), because
> > a simple boundary check is no longer sufficient.
> > 
> > The logic in swiotlb_alloc_tlb() is taken from __dma_direct_alloc_pages(),
> > simplified and enhanced to use coherent memory pools if needed.
> > 
> > Note that this is not the most efficient way to provide a bounce buffer,
> > but when a DMA buffer can't be mapped, something may (and will) actually
> > break. At that point it is better to make an allocation, even if it may be
> > an expensive operation.
> 
> I continue to think about swiotlb memory management from the standpoint
> of CoCo VMs that may be quite large with high network and storage loads.
> These VMs are often running mission-critical workloads that can't tolerate
> a bounce buffer allocation failure.  To prevent such failures, the swiotlb
> memory size must be overly large, which wastes memory.

If "mission critical workloads" are in a vm that allowes overcommit and
no control over other vms in that same system, then you have worse
problems, sorry.

Just don't do that.

thanks,

greg k-h

Re: [PATCH v3 1/7] swiotlb: make io_tlb_default_mem local to swiotlb.c

[Greg Kroah-Hartman]

On Tue, Jun 27, 2023 at 11:54:23AM +0200, Petr Tesarik wrote:
> +/**
> + * is_swiotlb_active() - check if the software IO TLB is initialized
> + * @dev: Device to check, or %NULL for the default IO TLB.
> + */
>  bool is_swiotlb_active(struct device *dev)
>  {
> - struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
> + struct io_tlb_mem *mem = dev
> + ? dev->dma_io_tlb_mem
> + : &io_tlb_default_mem;

That's impossible to read and maintain over time, sorry.

Please use real "if () else" lines, so that it can be maintained over
time.

thanks,

greg k-h

Re: [PATCH 08/11] sysctl: Add size to register_sysctl_init

[Greg Kroah-Hartman]

On Wed, Jun 21, 2023 at 11:09:57AM +0200, Joel Granados wrote:
>  static int __init random_sysctls_init(void)
>  {
> - register_sysctl_init("kernel/random", random_table);
> + register_sysctl_init("kernel/random", random_table,
> +  ARRAY_SIZE(random_table));

As mentioned before, why not just do:

#define register_sysctl_init(string, table) \
__register_sysctl_init(string, table, ARRAY_SIZE(table);

or something like that?

That way no callers need to change AND you prevent the size from ever
being incorrect?

thanks,

greg k-h

[PATCH v2 14/16] xen/xenbus: move to_xenbus_device() to use container_of_const()

[Greg Kroah-Hartman]

The driver core is changing to pass some pointers as const, so move
to_xenbus_device() to use container_of_const() to handle this change.

to_xenbus_device() now properly keeps the const-ness of the pointer passed
into it, while as before it could be lost.

Cc: Juergen Gross 
Cc: Stefano Stabellini 
Cc: Oleksandr Tyshchenko 
Cc: xen-devel@lists.xenproject.org
Signed-off-by: Greg Kroah-Hartman 
---
 include/xen/xenbus.h | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/include/xen/xenbus.h b/include/xen/xenbus.h
index eaa932b99d8a..b31f77f9c50c 100644
--- a/include/xen/xenbus.h
+++ b/include/xen/xenbus.h
@@ -96,10 +96,7 @@ struct xenbus_device {
unsigned int spurious_threshold;
 };
 
-static inline struct xenbus_device *to_xenbus_device(struct device *dev)
-{
-   return container_of(dev, struct xenbus_device, dev);
-}
+#define to_xenbus_device(__dev)container_of_const(__dev, struct 
xenbus_device, dev)
 
 struct xenbus_device_id
 {
-- 
2.39.0

Re: [PATCH 5.4] xen-netback: move removal of "hotplug-status" to the right place

[Greg Kroah-Hartman]

On Mon, Dec 19, 2022 at 04:37:10PM +0100, Pratyush Yadav wrote:
> The removal of "hotplug-status" has moved around a bit. First it was
> moved from netback_remove() to hotplug_status_changed() in upstream
> commit 1f2565780e9b ("xen-netback: remove 'hotplug-status' once it has
> served its purpose"). Then the change was reverted in upstream commit
> 0f4558ae9187 ("Revert "xen-netback: remove 'hotplug-status' once it has
> served its purpose""), but it moved the removal to backend_disconnect().
> Then the upstream commit c55f34b6aec2 ("xen-netback: only remove
> 'hotplug-status' when the vif is actually destroyed") moved it finally
> back to netback_remove(). The thing to note being it is removed
> unconditionally this time around.
> 
> The story on v5.4.y adds to this confusion. Commit 60e4e3198ce8 ("Revert
> "xen-netback: remove 'hotplug-status' once it has served its purpose"")
> is backported to v5.4.y but the original commit that it tries to revert
> was never present on 5.4. So the backport incorrectly ends up just
> adding another xenbus_rm() of "hotplug-status" in backend_disconnect().
> 
> Now in v5.4.y it is removed in both backend_disconnect() and
> netback_remove(). But it should only be removed in netback_remove(), as
> the upstream version does.
> 
> Removing "hotplug-status" in backend_disconnect() causes problems when
> the frontend unilaterally disconnects, as explained in
> c55f34b6aec2 ("xen-netback: only remove 'hotplug-status' when the vif is
> actually destroyed").
> 
> Remove "hotplug-status" in the same place as it is done on the upstream
> version to ensure unilateral re-connection of frontend continues to
> work.
> 
> Fixes: 60e4e3198ce8 ("Revert "xen-netback: remove 'hotplug-status' once it 
> has served its purpose"")
> Signed-off-by: Pratyush Yadav 
> ---
>  drivers/net/xen-netback/xenbus.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Now queued up, thanks.

greg k-h

Re: [PATCH 0/3] Stable backports of gntdev fixes

[Greg Kroah-Hartman]

On Sun, Oct 30, 2022 at 03:12:40AM -0400, Demi Marie Obenour wrote:
> I backported the recent gntdev patches to stable branches before 5.15.
> The first patch is a prerequisite for the other backports.  The second
> patch should apply cleanly to all stable branches, but the third only
> applies to 5.10 as it requires mmu_interval_notifier_insert_locked().

Patches 1 and 2 now queued up, see my comments on 3.

thanks,

greg k-h

Re: [PATCH 3/3] xen/gntdev: Accommodate VMA splitting

[Greg Kroah-Hartman]

On Sun, Oct 30, 2022 at 03:12:43AM -0400, Demi Marie Obenour wrote:
> From: "M. Vefa Bicakci" 
> 
> Prior to this commit, the gntdev driver code did not handle the
> following scenario correctly with paravirtualized (PV) Xen domains:

This is already in 5.10.152, do we need to add it again?

thanks,

greg k-h

Re: [PATCH 4.19 174/229] x86/entry: Work around Clang __bdos() bug

[Greg Kroah-Hartman]

On Mon, Oct 24, 2022 at 07:41:27PM +0200, Pavel Machek wrote:
> Hi!
> 
> > From: Kees Cook 
> > 
> > [ Upstream commit 3e1730842f142add55dc658929221521a9ea62b6 ]
> > 
> > Clang produces a false positive when building with CONFIG_FORTIFY_SOURCE=y
> > and CONFIG_UBSAN_BOUNDS=y when operating on an array with a dynamic
> > offset. Work around this by using a direct assignment of an empty
> > instance. Avoids this warning:
> > 
> > ../include/linux/fortify-string.h:309:4: warning: call to 
> > __write_overflow_field declared with 'warn
> > ing' attribute: detected write beyond size of field (1st parameter); maybe 
> > use struct_group()? [-Wat
> > tribute-warning]
> > __write_overflow_field(p_size_field, size);
> > ^
> > 
> > which was isolated to the memset() call in xen_load_idt().
> > 
> > Note that this looks very much like another bug that was worked around:
> > https://github.com/ClangBuiltLinux/linux/issues/1592
> 
> We don't have CONFIG_UBSAN_BOUNDS in 4.19, so maybe we don't need this
> one?

Good point, I'll drop this from 5.4.y and older now, thanks.

greg k-h

[PATCH 5.15 412/530] x86/entry: Work around Clang __bdos() bug

[Greg Kroah-Hartman]

From: Kees Cook 

[ Upstream commit 3e1730842f142add55dc658929221521a9ea62b6 ]

Clang produces a false positive when building with CONFIG_FORTIFY_SOURCE=y
and CONFIG_UBSAN_BOUNDS=y when operating on an array with a dynamic
offset. Work around this by using a direct assignment of an empty
instance. Avoids this warning:

../include/linux/fortify-string.h:309:4: warning: call to 
__write_overflow_field declared with 'warn
ing' attribute: detected write beyond size of field (1st parameter); maybe use 
struct_group()? [-Wat
tribute-warning]
__write_overflow_field(p_size_field, size);
^

which was isolated to the memset() call in xen_load_idt().

Note that this looks very much like another bug that was worked around:
https://github.com/ClangBuiltLinux/linux/issues/1592

Cc: Juergen Gross 
Cc: Boris Ostrovsky 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: Borislav Petkov 
Cc: Dave Hansen 
Cc: x...@kernel.org
Cc: "H. Peter Anvin" 
Cc: xen-devel@lists.xenproject.org
Reviewed-by: Boris Ostrovsky 
Link: 
https://lore.kernel.org/lkml/41527d69-e8ab-3f86-ff37-6b298c01d...@oracle.com
Signed-off-by: Kees Cook 
Signed-off-by: Sasha Levin 
---
 arch/x86/xen/enlighten_pv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index 133ef31639df..561aad13412f 100644
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -759,6 +759,7 @@ static void xen_load_idt(const struct desc_ptr *desc)
 {
static DEFINE_SPINLOCK(lock);
static struct trap_info traps[257];
+   static const struct trap_info zero = { };
unsigned out;
 
trace_xen_cpu_load_idt(desc);
@@ -768,7 +769,7 @@ static void xen_load_idt(const struct desc_ptr *desc)
memcpy(this_cpu_ptr(&idt_desc), desc, sizeof(idt_desc));
 
out = xen_convert_trap_info(desc, traps, false);
-   memset(&traps[out], 0, sizeof(traps[0]));
+   traps[out] = zero;
 
xen_mc_flush();
if (HYPERVISOR_set_trap_table(traps))
-- 
2.35.1

[PATCH 5.10 302/390] x86/entry: Work around Clang __bdos() bug

[Greg Kroah-Hartman]

From: Kees Cook 

[ Upstream commit 3e1730842f142add55dc658929221521a9ea62b6 ]

Clang produces a false positive when building with CONFIG_FORTIFY_SOURCE=y
and CONFIG_UBSAN_BOUNDS=y when operating on an array with a dynamic
offset. Work around this by using a direct assignment of an empty
instance. Avoids this warning:

../include/linux/fortify-string.h:309:4: warning: call to 
__write_overflow_field declared with 'warn
ing' attribute: detected write beyond size of field (1st parameter); maybe use 
struct_group()? [-Wat
tribute-warning]
__write_overflow_field(p_size_field, size);
^

which was isolated to the memset() call in xen_load_idt().

Note that this looks very much like another bug that was worked around:
https://github.com/ClangBuiltLinux/linux/issues/1592

Cc: Juergen Gross 
Cc: Boris Ostrovsky 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: Borislav Petkov 
Cc: Dave Hansen 
Cc: x...@kernel.org
Cc: "H. Peter Anvin" 
Cc: xen-devel@lists.xenproject.org
Reviewed-by: Boris Ostrovsky 
Link: 
https://lore.kernel.org/lkml/41527d69-e8ab-3f86-ff37-6b298c01d...@oracle.com
Signed-off-by: Kees Cook 
Signed-off-by: Sasha Levin 
---
 arch/x86/xen/enlighten_pv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index 804c65d2b95f..815030b7f6fa 100644
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -768,6 +768,7 @@ static void xen_load_idt(const struct desc_ptr *desc)
 {
static DEFINE_SPINLOCK(lock);
static struct trap_info traps[257];
+   static const struct trap_info zero = { };
unsigned out;
 
trace_xen_cpu_load_idt(desc);
@@ -777,7 +778,7 @@ static void xen_load_idt(const struct desc_ptr *desc)
memcpy(this_cpu_ptr(&idt_desc), desc, sizeof(idt_desc));
 
out = xen_convert_trap_info(desc, traps, false);
-   memset(&traps[out], 0, sizeof(traps[0]));
+   traps[out] = zero;
 
xen_mc_flush();
if (HYPERVISOR_set_trap_table(traps))
-- 
2.35.1

[PATCH 5.4 189/255] x86/entry: Work around Clang __bdos() bug

[Greg Kroah-Hartman]

From: Kees Cook 

[ Upstream commit 3e1730842f142add55dc658929221521a9ea62b6 ]

Clang produces a false positive when building with CONFIG_FORTIFY_SOURCE=y
and CONFIG_UBSAN_BOUNDS=y when operating on an array with a dynamic
offset. Work around this by using a direct assignment of an empty
instance. Avoids this warning:

../include/linux/fortify-string.h:309:4: warning: call to 
__write_overflow_field declared with 'warn
ing' attribute: detected write beyond size of field (1st parameter); maybe use 
struct_group()? [-Wat
tribute-warning]
__write_overflow_field(p_size_field, size);
^

which was isolated to the memset() call in xen_load_idt().

Note that this looks very much like another bug that was worked around:
https://github.com/ClangBuiltLinux/linux/issues/1592

Cc: Juergen Gross 
Cc: Boris Ostrovsky 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: Borislav Petkov 
Cc: Dave Hansen 
Cc: x...@kernel.org
Cc: "H. Peter Anvin" 
Cc: xen-devel@lists.xenproject.org
Reviewed-by: Boris Ostrovsky 
Link: 
https://lore.kernel.org/lkml/41527d69-e8ab-3f86-ff37-6b298c01d...@oracle.com
Signed-off-by: Kees Cook 
Signed-off-by: Sasha Levin 
---
 arch/x86/xen/enlighten_pv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index 65cf405cd975..26b35e75b701 100644
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -759,6 +759,7 @@ static void xen_load_idt(const struct desc_ptr *desc)
 {
static DEFINE_SPINLOCK(lock);
static struct trap_info traps[257];
+   static const struct trap_info zero = { };
unsigned out;
 
trace_xen_cpu_load_idt(desc);
@@ -768,7 +769,7 @@ static void xen_load_idt(const struct desc_ptr *desc)
memcpy(this_cpu_ptr(&idt_desc), desc, sizeof(idt_desc));
 
out = xen_convert_trap_info(desc, traps, false);
-   memset(&traps[out], 0, sizeof(traps[0]));
+   traps[out] = zero;
 
xen_mc_flush();
if (HYPERVISOR_set_trap_table(traps))
-- 
2.35.1

[PATCH 4.19 174/229] x86/entry: Work around Clang __bdos() bug

[Greg Kroah-Hartman]

From: Kees Cook 

[ Upstream commit 3e1730842f142add55dc658929221521a9ea62b6 ]

Clang produces a false positive when building with CONFIG_FORTIFY_SOURCE=y
and CONFIG_UBSAN_BOUNDS=y when operating on an array with a dynamic
offset. Work around this by using a direct assignment of an empty
instance. Avoids this warning:

../include/linux/fortify-string.h:309:4: warning: call to 
__write_overflow_field declared with 'warn
ing' attribute: detected write beyond size of field (1st parameter); maybe use 
struct_group()? [-Wat
tribute-warning]
__write_overflow_field(p_size_field, size);
^

which was isolated to the memset() call in xen_load_idt().

Note that this looks very much like another bug that was worked around:
https://github.com/ClangBuiltLinux/linux/issues/1592

Cc: Juergen Gross 
Cc: Boris Ostrovsky 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: Borislav Petkov 
Cc: Dave Hansen 
Cc: x...@kernel.org
Cc: "H. Peter Anvin" 
Cc: xen-devel@lists.xenproject.org
Reviewed-by: Boris Ostrovsky 
Link: 
https://lore.kernel.org/lkml/41527d69-e8ab-3f86-ff37-6b298c01d...@oracle.com
Signed-off-by: Kees Cook 
Signed-off-by: Sasha Levin 
---
 arch/x86/xen/enlighten_pv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index 8f1ff8dad2ce..04bfd9c3987b 100644
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -752,6 +752,7 @@ static void xen_load_idt(const struct desc_ptr *desc)
 {
static DEFINE_SPINLOCK(lock);
static struct trap_info traps[257];
+   static const struct trap_info zero = { };
unsigned out;
 
trace_xen_cpu_load_idt(desc);
@@ -761,7 +762,7 @@ static void xen_load_idt(const struct desc_ptr *desc)
memcpy(this_cpu_ptr(&idt_desc), desc, sizeof(idt_desc));
 
out = xen_convert_trap_info(desc, traps, false);
-   memset(&traps[out], 0, sizeof(traps[0]));
+   traps[out] = zero;
 
xen_mc_flush();
if (HYPERVISOR_set_trap_table(traps))
-- 
2.35.1

[PATCH 4.14 165/210] x86/entry: Work around Clang __bdos() bug

[Greg Kroah-Hartman]

From: Kees Cook 

[ Upstream commit 3e1730842f142add55dc658929221521a9ea62b6 ]

Clang produces a false positive when building with CONFIG_FORTIFY_SOURCE=y
and CONFIG_UBSAN_BOUNDS=y when operating on an array with a dynamic
offset. Work around this by using a direct assignment of an empty
instance. Avoids this warning:

../include/linux/fortify-string.h:309:4: warning: call to 
__write_overflow_field declared with 'warn
ing' attribute: detected write beyond size of field (1st parameter); maybe use 
struct_group()? [-Wat
tribute-warning]
__write_overflow_field(p_size_field, size);
^

which was isolated to the memset() call in xen_load_idt().

Note that this looks very much like another bug that was worked around:
https://github.com/ClangBuiltLinux/linux/issues/1592

Cc: Juergen Gross 
Cc: Boris Ostrovsky 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: Borislav Petkov 
Cc: Dave Hansen 
Cc: x...@kernel.org
Cc: "H. Peter Anvin" 
Cc: xen-devel@lists.xenproject.org
Reviewed-by: Boris Ostrovsky 
Link: 
https://lore.kernel.org/lkml/41527d69-e8ab-3f86-ff37-6b298c01d...@oracle.com
Signed-off-by: Kees Cook 
Signed-off-by: Sasha Levin 
---
 arch/x86/xen/enlighten_pv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index 88d084a57b14..3f38541c28d2 100644
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -753,6 +753,7 @@ static void xen_load_idt(const struct desc_ptr *desc)
 {
static DEFINE_SPINLOCK(lock);
static struct trap_info traps[257];
+   static const struct trap_info zero = { };
unsigned out;
 
trace_xen_cpu_load_idt(desc);
@@ -762,7 +763,7 @@ static void xen_load_idt(const struct desc_ptr *desc)
memcpy(this_cpu_ptr(&idt_desc), desc, sizeof(idt_desc));
 
out = xen_convert_trap_info(desc, traps, false);
-   memset(&traps[out], 0, sizeof(traps[0]));
+   traps[out] = zero;
 
xen_mc_flush();
if (HYPERVISOR_set_trap_table(traps))
-- 
2.35.1

[PATCH 5.19 553/717] x86/entry: Work around Clang __bdos() bug

[Greg Kroah-Hartman]

From: Kees Cook 

[ Upstream commit 3e1730842f142add55dc658929221521a9ea62b6 ]

Clang produces a false positive when building with CONFIG_FORTIFY_SOURCE=y
and CONFIG_UBSAN_BOUNDS=y when operating on an array with a dynamic
offset. Work around this by using a direct assignment of an empty
instance. Avoids this warning:

../include/linux/fortify-string.h:309:4: warning: call to 
__write_overflow_field declared with 'warn
ing' attribute: detected write beyond size of field (1st parameter); maybe use 
struct_group()? [-Wat
tribute-warning]
__write_overflow_field(p_size_field, size);
^

which was isolated to the memset() call in xen_load_idt().

Note that this looks very much like another bug that was worked around:
https://github.com/ClangBuiltLinux/linux/issues/1592

Cc: Juergen Gross 
Cc: Boris Ostrovsky 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: Borislav Petkov 
Cc: Dave Hansen 
Cc: x...@kernel.org
Cc: "H. Peter Anvin" 
Cc: xen-devel@lists.xenproject.org
Reviewed-by: Boris Ostrovsky 
Link: 
https://lore.kernel.org/lkml/41527d69-e8ab-3f86-ff37-6b298c01d...@oracle.com
Signed-off-by: Kees Cook 
Signed-off-by: Sasha Levin 
---
 arch/x86/xen/enlighten_pv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index 0ed2e487a693..9b1a58dda935 100644
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -765,6 +765,7 @@ static void xen_load_idt(const struct desc_ptr *desc)
 {
static DEFINE_SPINLOCK(lock);
static struct trap_info traps[257];
+   static const struct trap_info zero = { };
unsigned out;
 
trace_xen_cpu_load_idt(desc);
@@ -774,7 +775,7 @@ static void xen_load_idt(const struct desc_ptr *desc)
memcpy(this_cpu_ptr(&idt_desc), desc, sizeof(idt_desc));
 
out = xen_convert_trap_info(desc, traps, false);
-   memset(&traps[out], 0, sizeof(traps[0]));
+   traps[out] = zero;
 
xen_mc_flush();
if (HYPERVISOR_set_trap_table(traps))
-- 
2.35.1

[PATCH 6.0 676/862] x86/entry: Work around Clang __bdos() bug

[Greg Kroah-Hartman]

From: Kees Cook 

[ Upstream commit 3e1730842f142add55dc658929221521a9ea62b6 ]

Clang produces a false positive when building with CONFIG_FORTIFY_SOURCE=y
and CONFIG_UBSAN_BOUNDS=y when operating on an array with a dynamic
offset. Work around this by using a direct assignment of an empty
instance. Avoids this warning:

../include/linux/fortify-string.h:309:4: warning: call to 
__write_overflow_field declared with 'warn
ing' attribute: detected write beyond size of field (1st parameter); maybe use 
struct_group()? [-Wat
tribute-warning]
__write_overflow_field(p_size_field, size);
^

which was isolated to the memset() call in xen_load_idt().

Note that this looks very much like another bug that was worked around:
https://github.com/ClangBuiltLinux/linux/issues/1592

Cc: Juergen Gross 
Cc: Boris Ostrovsky 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: Borislav Petkov 
Cc: Dave Hansen 
Cc: x...@kernel.org
Cc: "H. Peter Anvin" 
Cc: xen-devel@lists.xenproject.org
Reviewed-by: Boris Ostrovsky 
Link: 
https://lore.kernel.org/lkml/41527d69-e8ab-3f86-ff37-6b298c01d...@oracle.com
Signed-off-by: Kees Cook 
Signed-off-by: Sasha Levin 
---
 arch/x86/xen/enlighten_pv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index 0ed2e487a693..9b1a58dda935 100644
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -765,6 +765,7 @@ static void xen_load_idt(const struct desc_ptr *desc)
 {
static DEFINE_SPINLOCK(lock);
static struct trap_info traps[257];
+   static const struct trap_info zero = { };
unsigned out;
 
trace_xen_cpu_load_idt(desc);
@@ -774,7 +775,7 @@ static void xen_load_idt(const struct desc_ptr *desc)
memcpy(this_cpu_ptr(&idt_desc), desc, sizeof(idt_desc));
 
out = xen_convert_trap_info(desc, traps, false);
-   memset(&traps[out], 0, sizeof(traps[0]));
+   traps[out] = zero;
 
xen_mc_flush();
if (HYPERVISOR_set_trap_table(traps))
-- 
2.35.1

Re: Backport request

[Greg Kroah-Hartman]

On Wed, Aug 24, 2022 at 03:52:27PM +0200, Juergen Gross wrote:
> On 24.08.22 14:10, Greg Kroah-Hartman wrote:
> > On Wed, Aug 24, 2022 at 01:20:22PM +0200, Juergen Gross wrote:
> > > Hi Greg,
> > > 
> > > stable kernels 5.18 and 5.15 seem to be missing upstream patch
> > > c64cc2802a78 ("x86/entry: Move CLD to the start of the idtentry macro").
> > > This is a prerequisite patch for 64cbd0acb582 ("x86/entry: Don't call
> > > error_entry() for XENPV"), which is included in 5.15.y and 5.18.y.
> > > 
> > > Could you please take c64cc2802a78 for 5.15 and 5.18?
> > 
> > 5.18 is end-of-life, so that's impossible to do now :(
> > 
> > For 5.15.y, the commit does not apply cleanly, can you provide a working
> > backport?
> 
> Attached.

Thanks, now queued up.

greg k-h

Re: Backport request

[Greg Kroah-Hartman]

On Wed, Aug 24, 2022 at 01:20:22PM +0200, Juergen Gross wrote:
> Hi Greg,
> 
> stable kernels 5.18 and 5.15 seem to be missing upstream patch
> c64cc2802a78 ("x86/entry: Move CLD to the start of the idtentry macro").
> This is a prerequisite patch for 64cbd0acb582 ("x86/entry: Don't call
> error_entry() for XENPV"), which is included in 5.15.y and 5.18.y.
> 
> Could you please take c64cc2802a78 for 5.15 and 5.18?

5.18 is end-of-life, so that's impossible to do now :(

For 5.15.y, the commit does not apply cleanly, can you provide a working
backport?

thanks,

greg k-h

Re: Backport of 166d38632316 ("xen/gntdev: Ignore failure to unmap INVALID_GRANT_HANDLE")

[Greg Kroah-Hartman]

On Fri, Jul 22, 2022 at 11:44:10PM -0400, Demi Marie Obenour wrote:
> This series backports upstream commit 166d3863231667c4f64dee72b77d1102cdfad11f
> to all supported stable kernel trees.

All now queued up, thanks.

greg k-h

[PATCH 5.4 13/18] x86: kprobes: Prohibit probing on instruction which has emulate prefix

[Greg Kroah-Hartman]

From: Masami Hiramatsu 

commit 004e8dce9c5595697951f7cd0e9f66b35c92265e upstream.

Prohibit probing on instruction which has XEN_EMULATE_PREFIX
or KVM's emulate prefix. Since that prefix is a marker for Xen
and KVM, if we modify the marker by kprobe's int3, that doesn't
work as expected.

Signed-off-by: Masami Hiramatsu 
Signed-off-by: Peter Zijlstra (Intel) 
Cc: Juergen Gross 
Cc: x...@kernel.org
Cc: Boris Ostrovsky 
Cc: Ingo Molnar 
Cc: Stefano Stabellini 
Cc: Andrew Cooper 
Cc: Borislav Petkov 
Cc: xen-devel@lists.xenproject.org
Cc: Randy Dunlap 
Cc: Josh Poimboeuf 
Link: 
https://lkml.kernel.org/r/156777566048.25081.6296162369492175325.stgit@devnote2
Signed-off-by: Maximilian Heyne 
Signed-off-by: Greg Kroah-Hartman 
---
 arch/x86/kernel/kprobes/core.c |4 
 1 file changed, 4 insertions(+)

--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -358,6 +358,10 @@ int __copy_instruction(u8 *dest, u8 *src
kernel_insn_init(insn, dest, MAX_INSN_SIZE);
insn_get_length(insn);
 
+   /* We can not probe force emulate prefixed instruction */
+   if (insn_has_emulate_prefix(insn))
+   return 0;
+
/* Another subsystem puts a breakpoint, failed to recover */
if (insn->opcode.bytes[0] == BREAKPOINT_INSTRUCTION)
return 0;

[PATCH 5.4 12/18] x86: xen: insn: Decode Xen and KVM emulate-prefix signature

[Greg Kroah-Hartman]

From: Masami Hiramatsu 

commit 4d65adfcd1196818659d3bd9b42dccab291e1751 upstream.

Decode Xen and KVM's emulate-prefix signature by x86 insn decoder.
It is called "prefix" but actually not x86 instruction prefix, so
this adds insn.emulate_prefix_size field instead of reusing
insn.prefixes.

If x86 decoder finds a special sequence of instructions of
XEN_EMULATE_PREFIX and 'ud2a; .ascii "kvm"', it just counts the
length, set insn.emulate_prefix_size and fold it with the next
instruction. In other words, the signature and the next instruction
is treated as a single instruction.

Signed-off-by: Masami Hiramatsu 
Signed-off-by: Peter Zijlstra (Intel) 
Acked-by: Josh Poimboeuf 
Cc: Juergen Gross 
Cc: x...@kernel.org
Cc: Boris Ostrovsky 
Cc: Ingo Molnar 
Cc: Stefano Stabellini 
Cc: Andrew Cooper 
Cc: Borislav Petkov 
Cc: xen-devel@lists.xenproject.org
Cc: Randy Dunlap 
Link: 
https://lkml.kernel.org/r/156777564986.25081.4964537658500952557.stgit@devnote2
[mheyne: resolved contextual conflict in tools/objtools/sync-check.sh]
Signed-off-by: Maximilian Heyne 
Signed-off-by: Greg Kroah-Hartman 
---
 arch/x86/include/asm/insn.h |6 
 arch/x86/lib/insn.c |   34 
 tools/arch/x86/include/asm/emulate_prefix.h |   14 +++
 tools/arch/x86/include/asm/insn.h   |6 
 tools/arch/x86/lib/insn.c   |   34 
 tools/objtool/sync-check.sh |3 +-
 tools/perf/check-headers.sh |3 +-
 7 files changed, 98 insertions(+), 2 deletions(-)
 create mode 100644 tools/arch/x86/include/asm/emulate_prefix.h

--- a/arch/x86/include/asm/insn.h
+++ b/arch/x86/include/asm/insn.h
@@ -45,6 +45,7 @@ struct insn {
struct insn_field immediate2;   /* for 64bit imm or seg16 */
};
 
+   int emulate_prefix_size;
insn_attr_t attr;
unsigned char opnd_bytes;
unsigned char addr_bytes;
@@ -128,6 +129,11 @@ static inline int insn_is_evex(struct in
return (insn->vex_prefix.nbytes == 4);
 }
 
+static inline int insn_has_emulate_prefix(struct insn *insn)
+{
+   return !!insn->emulate_prefix_size;
+}
+
 /* Ensure this instruction is decoded completely */
 static inline int insn_complete(struct insn *insn)
 {
--- a/arch/x86/lib/insn.c
+++ b/arch/x86/lib/insn.c
@@ -13,6 +13,8 @@
 #include 
 #include 
 
+#include 
+
 /* Verify next sizeof(t) bytes can be on the same instruction */
 #define validate_next(t, insn, n)  \
((insn)->next_byte + sizeof(t) + n <= (insn)->end_kaddr)
@@ -58,6 +60,36 @@ void insn_init(struct insn *insn, const
insn->addr_bytes = 4;
 }
 
+static const insn_byte_t xen_prefix[] = { __XEN_EMULATE_PREFIX };
+static const insn_byte_t kvm_prefix[] = { __KVM_EMULATE_PREFIX };
+
+static int __insn_get_emulate_prefix(struct insn *insn,
+const insn_byte_t *prefix, size_t len)
+{
+   size_t i;
+
+   for (i = 0; i < len; i++) {
+   if (peek_nbyte_next(insn_byte_t, insn, i) != prefix[i])
+   goto err_out;
+   }
+
+   insn->emulate_prefix_size = len;
+   insn->next_byte += len;
+
+   return 1;
+
+err_out:
+   return 0;
+}
+
+static void insn_get_emulate_prefix(struct insn *insn)
+{
+   if (__insn_get_emulate_prefix(insn, xen_prefix, sizeof(xen_prefix)))
+   return;
+
+   __insn_get_emulate_prefix(insn, kvm_prefix, sizeof(kvm_prefix));
+}
+
 /**
  * insn_get_prefixes - scan x86 instruction prefix bytes
  * @insn:  &struct insn containing instruction
@@ -76,6 +108,8 @@ void insn_get_prefixes(struct insn *insn
if (prefixes->got)
return;
 
+   insn_get_emulate_prefix(insn);
+
nb = 0;
lb = 0;
b = peek_next(insn_byte_t, insn);
--- /dev/null
+++ b/tools/arch/x86/include/asm/emulate_prefix.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_X86_EMULATE_PREFIX_H
+#define _ASM_X86_EMULATE_PREFIX_H
+
+/*
+ * Virt escape sequences to trigger instruction emulation;
+ * ideally these would decode to 'whole' instruction and not destroy
+ * the instruction stream; sadly this is not true for the 'kvm' one :/
+ */
+
+#define __XEN_EMULATE_PREFIX  0x0f,0x0b,0x78,0x65,0x6e  /* ud2 ; .ascii "xen" 
*/
+#define __KVM_EMULATE_PREFIX  0x0f,0x0b,0x6b,0x76,0x6d /* ud2 ; .ascii "kvm" */
+
+#endif
--- a/tools/arch/x86/include/asm/insn.h
+++ b/tools/arch/x86/include/asm/insn.h
@@ -45,6 +45,7 @@ struct insn {
struct insn_field immediate2;   /* for 64bit imm or seg16 */
};
 
+   int emulate_prefix_size;
insn_attr_t attr;
unsigned char opnd_bytes;
unsigned char addr_bytes;
@@ -128,6 +129,11 @@ static inline int insn_is_evex(struct in
return (insn->vex_

[PATCH 5.4 11/18] x86: xen: kvm: Gather the definition of emulate prefixes

[Greg Kroah-Hartman]

From: Masami Hiramatsu 

commit b3dc0695fa40c3b280230fb6fb7fb7a94ce28bf4 upstream.

Gather the emulate prefixes, which forcibly make the following
instruction emulated on virtualization, in one place.

Suggested-by: Peter Zijlstra 
Signed-off-by: Masami Hiramatsu 
Signed-off-by: Peter Zijlstra (Intel) 
Cc: Juergen Gross 
Cc: x...@kernel.org
Cc: Ingo Molnar 
Cc: Boris Ostrovsky 
Cc: Andrew Cooper 
Cc: Stefano Stabellini 
Cc: Borislav Petkov 
Cc: xen-devel@lists.xenproject.org
Cc: Randy Dunlap 
Cc: Josh Poimboeuf 
Link: 
https://lkml.kernel.org/r/156777563917.25081.7286628561790289995.stgit@devnote2
Signed-off-by: Maximilian Heyne 
Signed-off-by: Greg Kroah-Hartman 
---
 arch/x86/include/asm/emulate_prefix.h |   14 ++
 arch/x86/include/asm/xen/interface.h  |   11 ---
 arch/x86/kvm/x86.c|4 +++-
 3 files changed, 21 insertions(+), 8 deletions(-)
 create mode 100644 arch/x86/include/asm/emulate_prefix.h

--- /dev/null
+++ b/arch/x86/include/asm/emulate_prefix.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_X86_EMULATE_PREFIX_H
+#define _ASM_X86_EMULATE_PREFIX_H
+
+/*
+ * Virt escape sequences to trigger instruction emulation;
+ * ideally these would decode to 'whole' instruction and not destroy
+ * the instruction stream; sadly this is not true for the 'kvm' one :/
+ */
+
+#define __XEN_EMULATE_PREFIX  0x0f,0x0b,0x78,0x65,0x6e  /* ud2 ; .ascii "xen" 
*/
+#define __KVM_EMULATE_PREFIX  0x0f,0x0b,0x6b,0x76,0x6d /* ud2 ; .ascii "kvm" */
+
+#endif
--- a/arch/x86/include/asm/xen/interface.h
+++ b/arch/x86/include/asm/xen/interface.h
@@ -379,12 +379,9 @@ struct xen_pmu_arch {
  * Prefix forces emulation of some non-trapping instructions.
  * Currently only CPUID.
  */
-#ifdef __ASSEMBLY__
-#define XEN_EMULATE_PREFIX .byte 0x0f,0x0b,0x78,0x65,0x6e ;
-#define XEN_CPUID  XEN_EMULATE_PREFIX cpuid
-#else
-#define XEN_EMULATE_PREFIX ".byte 0x0f,0x0b,0x78,0x65,0x6e ; "
-#define XEN_CPUID  XEN_EMULATE_PREFIX "cpuid"
-#endif
+#include 
+
+#define XEN_EMULATE_PREFIX __ASM_FORM(.byte __XEN_EMULATE_PREFIX ;)
+#define XEN_CPUID  XEN_EMULATE_PREFIX __ASM_FORM(cpuid)
 
 #endif /* _ASM_X86_XEN_INTERFACE_H */
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -68,6 +68,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #define CREATE_TRACE_POINTS
@@ -5583,6 +5584,7 @@ EXPORT_SYMBOL_GPL(kvm_write_guest_virt_s
 
 int handle_ud(struct kvm_vcpu *vcpu)
 {
+   static const char kvm_emulate_prefix[] = { __KVM_EMULATE_PREFIX };
int emul_type = EMULTYPE_TRAP_UD;
char sig[5]; /* ud2; .ascii "kvm" */
struct x86_exception e;
@@ -5590,7 +5592,7 @@ int handle_ud(struct kvm_vcpu *vcpu)
if (force_emulation_prefix &&
kvm_read_guest_virt(vcpu, kvm_get_linear_rip(vcpu),
sig, sizeof(sig), &e) == 0 &&
-   memcmp(sig, "\xf\xbkvm", sizeof(sig)) == 0) {
+   memcmp(sig, kvm_emulate_prefix, sizeof(sig)) == 0) {
kvm_rip_write(vcpu, kvm_rip_read(vcpu) + sizeof(sig));
emul_type = EMULTYPE_TRAP_UD_FORCED;
}

[PATCH 5.4 10/18] x86/asm: Allow to pass macros to __ASM_FORM()

[Greg Kroah-Hartman]

From: Masami Hiramatsu 

commit f7919fd943abf0c77aed4441ea9897a323d132f5 upstream.

Use __stringify() at __ASM_FORM() so that user can pass
code including macros to __ASM_FORM().

Signed-off-by: Masami Hiramatsu 
Signed-off-by: Peter Zijlstra (Intel) 
Cc: Juergen Gross 
Cc: x...@kernel.org
Cc: Boris Ostrovsky 
Cc: Ingo Molnar 
Cc: Stefano Stabellini 
Cc: Andrew Cooper 
Cc: Borislav Petkov 
Cc: xen-devel@lists.xenproject.org
Cc: Randy Dunlap 
Cc: Josh Poimboeuf 
Link: 
https://lkml.kernel.org/r/156777562873.25081.2288083344657460959.stgit@devnote2
Signed-off-by: Maximilian Heyne 
Signed-off-by: Greg Kroah-Hartman 
---
 arch/x86/include/asm/asm.h |8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

--- a/arch/x86/include/asm/asm.h
+++ b/arch/x86/include/asm/asm.h
@@ -7,9 +7,11 @@
 # define __ASM_FORM_RAW(x) x
 # define __ASM_FORM_COMMA(x) x,
 #else
-# define __ASM_FORM(x) " " #x " "
-# define __ASM_FORM_RAW(x) #x
-# define __ASM_FORM_COMMA(x) " " #x ","
+#include 
+
+# define __ASM_FORM(x) " " __stringify(x) " "
+# define __ASM_FORM_RAW(x) __stringify(x)
+# define __ASM_FORM_COMMA(x) " " __stringify(x) ","
 #endif
 
 #ifndef __x86_64__

Re: [PATCH 16/18] xen/usbfront: use xenbus_setup_ring() and xenbus_teardown_ring()

[Greg Kroah-Hartman]

On Wed, Apr 20, 2022 at 05:09:40PM +0200, Juergen Gross wrote:
> Simplify xen-hcd's ring creation and removal via xenbus_setup_ring()
> and xenbus_teardown_ring().
> 
> Signed-off-by: Juergen Gross 
> ---
>  drivers/usb/host/xen-hcd.c | 55 +-
>  1 file changed, 13 insertions(+), 42 deletions(-)

Acked-by: Greg Kroah-Hartman 

Re: [PATCH 04/18] xen/usb: switch xen-hcd to use INVALID_GRANT_REF

[Greg Kroah-Hartman]

On Wed, Apr 20, 2022 at 05:09:28PM +0200, Juergen Gross wrote:
> Instead of using a private macro for an invalid grant reference use
> the common one.
> 
> Signed-off-by: Juergen Gross 
> ---
>  drivers/usb/host/xen-hcd.c | 14 ++
>  1 file changed, 6 insertions(+), 8 deletions(-)

Acked-by: Greg Kroah-Hartman 

Re: [PATCH] xen/usb: harden xen_hcd against malicious backends

[Greg Kroah-Hartman]

On Wed, Mar 16, 2022 at 06:29:00AM +0100, Juergen Gross wrote:
> On 15.03.22 18:41, Greg Kroah-Hartman wrote:
> > On Fri, Mar 11, 2022 at 11:35:09AM +0100, Juergen Gross wrote:
> > > Make sure a malicious backend can't cause any harm other than wrong
> > > I/O data.
> > > 
> > > Missing are verification of the request id in a response, sanitizing
> > > the reported actual I/O length, and protection against interrupt storms
> > > from the backend.
> > > 
> > > Signed-off-by: Juergen Gross 
> > > ---
> > >   drivers/usb/host/xen-hcd.c | 57 --
> > >   1 file changed, 43 insertions(+), 14 deletions(-)
> > 
> > Fails to apply to my tree:
> > 
> > checking file drivers/usb/host/xen-hcd.c
> > Hunk #2 succeeded at 720 (offset -1 lines).
> > Hunk #3 succeeded at 807 (offset -3 lines).
> > Hunk #4 succeeded at 934 (offset -5 lines).
> > Hunk #5 FAILED at 986.
> > Hunk #6 succeeded at 1003 with fuzz 1 (offset -10 lines).
> > Hunk #7 succeeded at 1048 (offset -10 lines).
> > Hunk #8 succeeded at 1072 (offset -10 lines).
> > Hunk #9 succeeded at 1161 (offset -10 lines).
> > Hunk #10 succeeded at 1516 (offset -10 lines).
> > 1 out of 10 hunks FAILED
> > 
> > Any hints?
> 
> Rebase your tree to v5.17-rc8? It is missing the recent security
> patches which modified drivers/usb/host/xen-hcd.c.

I can't rebase, but I can merge.  I'll do that, thanks.

greg k-h

Re: [PATCH] xen/usb: harden xen_hcd against malicious backends

[Greg Kroah-Hartman]

On Fri, Mar 11, 2022 at 11:35:09AM +0100, Juergen Gross wrote:
> Make sure a malicious backend can't cause any harm other than wrong
> I/O data.
> 
> Missing are verification of the request id in a response, sanitizing
> the reported actual I/O length, and protection against interrupt storms
> from the backend.
> 
> Signed-off-by: Juergen Gross 
> ---
>  drivers/usb/host/xen-hcd.c | 57 --
>  1 file changed, 43 insertions(+), 14 deletions(-)

Fails to apply to my tree:

checking file drivers/usb/host/xen-hcd.c
Hunk #2 succeeded at 720 (offset -1 lines).
Hunk #3 succeeded at 807 (offset -3 lines).
Hunk #4 succeeded at 934 (offset -5 lines).
Hunk #5 FAILED at 986.
Hunk #6 succeeded at 1003 with fuzz 1 (offset -10 lines).
Hunk #7 succeeded at 1048 (offset -10 lines).
Hunk #8 succeeded at 1072 (offset -10 lines).
Hunk #9 succeeded at 1161 (offset -10 lines).
Hunk #10 succeeded at 1516 (offset -10 lines).
1 out of 10 hunks FAILED

Any hints?

thanks,

greg k-h

Re: [PATCH] drivers/xen: use helper macro __ATTR_RW

[Greg Kroah-Hartman]

On Sat, Mar 05, 2022 at 11:33:58PM +0800, zhanglianjie wrote:
> Hi,
> 
> 在 2022/3/5 21:55, Greg Kroah-Hartman 写道:
> > On Sat, Mar 05, 2022 at 09:38:23PM +0800, zhanglianjie wrote:
> > > Use helper macro __ATTR_RW to define HYPERVISOR_ATTR_RW to make code more 
> > > clear.
> > > Minor readability improvement.
> > > 
> > > Signed-off-by: zhanglianjie 
> > 
> > Is this the name you sign legal documents with?  (I have to ask...)
> Yes, this is my signature.
> > 
> > > 
> > > diff --git a/drivers/xen/sys-hypervisor.c b/drivers/xen/sys-hypervisor.c
> > > index feb1d16252e7..fcb0792f090e 100644
> > > --- a/drivers/xen/sys-hypervisor.c
> > > +++ b/drivers/xen/sys-hypervisor.c
> > > @@ -22,11 +22,10 @@
> > >   #endif
> > > 
> > >   #define HYPERVISOR_ATTR_RO(_name) \
> > > -static struct hyp_sysfs_attr  _name##_attr = __ATTR_RO(_name)
> > > +static struct hyp_sysfs_attr _name##_attr = __ATTR_RO(_name)
> > 
> > Why change this line?  That's not relevant to this change :(
> > 
> There are two spaces between struct hyp_sysfs_attr and _name, by the way,
> modify it.

Coding style cleanups should be a separate patch, as each change should
only do one logical thing.

But hey, I'm not the maintainer of this file/driver, others might be
more lax.

good luck!

greg k-h

Re: [PATCH] drivers/xen: use helper macro __ATTR_RW

[Greg Kroah-Hartman]

On Sat, Mar 05, 2022 at 09:38:23PM +0800, zhanglianjie wrote:
> Use helper macro __ATTR_RW to define HYPERVISOR_ATTR_RW to make code more 
> clear.
> Minor readability improvement.
> 
> Signed-off-by: zhanglianjie 

Is this the name you sign legal documents with?  (I have to ask...)

> 
> diff --git a/drivers/xen/sys-hypervisor.c b/drivers/xen/sys-hypervisor.c
> index feb1d16252e7..fcb0792f090e 100644
> --- a/drivers/xen/sys-hypervisor.c
> +++ b/drivers/xen/sys-hypervisor.c
> @@ -22,11 +22,10 @@
>  #endif
> 
>  #define HYPERVISOR_ATTR_RO(_name) \
> -static struct hyp_sysfs_attr  _name##_attr = __ATTR_RO(_name)
> +static struct hyp_sysfs_attr _name##_attr = __ATTR_RO(_name)

Why change this line?  That's not relevant to this change :(

thanks,

greg k-h

Re: [patch] genirq/msi: Populate sysfs entry only once

[Greg Kroah-Hartman]

On Mon, Jan 10, 2022 at 07:12:45PM +0100, Thomas Gleixner wrote:
> The MSI entries for multi-MSI are populated en bloc for the MSI descriptor,
> but the current code invokes the population inside the per interrupt loop
> which triggers a warning in the sysfs code and causes the interrupt
> allocation to fail.
> 
> Move it outside of the loop so it works correctly for single and multi-MSI.
> 
> Fixes: bf5e758f02fc ("genirq/msi: Simplify sysfs handling")
> Reported-by: Borislav Petkov 
> Signed-off-by: Thomas Gleixner 
> ---
>  kernel/irq/msi.c |   11 +--
>  1 file changed, 5 insertions(+), 6 deletions(-)

Reviewed-by: Greg Kroah-Hartman 

[PATCH 4.4 12/17] xen/blkfront: fix bug in backported patch

[Greg Kroah-Hartman]

From: Juergen Gross 

The backport of commit 8f5a695d99000fc ("xen/blkfront: don't take local
copy of a request from the ring page") to stable 4.4 kernel introduced
a bug when adding the needed blkif_ring_get_request() function, as
info->ring.req_prod_pvt was incremented twice now.

Fix that be deleting the now superfluous increments after calling that
function.

Signed-off-by: Juergen Gross 
Signed-off-by: Greg Kroah-Hartman 
---
 drivers/block/xen-blkfront.c |4 
 1 file changed, 4 deletions(-)

--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -493,8 +493,6 @@ static int blkif_queue_discard_req(struc
else
ring_req->u.discard.flag = 0;
 
-   info->ring.req_prod_pvt++;
-
/* Copy the request to the ring page. */
*final_ring_req = *ring_req;
info->shadow[id].inflight = true;
@@ -711,8 +709,6 @@ static int blkif_queue_rw_req(struct req
if (setup.segments)
kunmap_atomic(setup.segments);
 
-   info->ring.req_prod_pvt++;
-
/* Copy request(s) to the ring page. */
*final_ring_req = *ring_req;
info->shadow[id].inflight = true;

Re: [patch V4 09-02/35] PCI/MSI: Allocate MSI device data on first use

[Greg Kroah-Hartman]

On Wed, Dec 15, 2021 at 06:19:49PM +0100, Thomas Gleixner wrote:
> Allocate MSI device data on first use, i.e. when a PCI driver invokes one
> of the PCI/MSI enablement functions.
> 
> Add a wrapper function to ensure that the ordering vs. pcim_msi_release()
> is correct.
> 
> Signed-off-by: Thomas Gleixner 

Reviewed-by: Greg Kroah-Hartman 

Re: [patch V4 09-01/35] PCI/MSI: Decouple MSI[-X] disable from pcim_release()

[Greg Kroah-Hartman]

On Wed, Dec 15, 2021 at 06:16:44PM +0100, Thomas Gleixner wrote:
> The MSI core will introduce runtime allocation of MSI related data. This
> data will be devres managed and has to be set up before enabling
> PCI/MSI[-X]. This would introduce an ordering issue vs. pcim_release().
> 
> The setup order is:
> 
>pcim_enable_device()
>   devres_alloc(pcim_release...);
>   ...
>   pci_irq_alloc()
> msi_setup_device_data()
>devres_alloc(msi_device_data_release, ...)
> 
> and once the device is released these release functions are invoked in the
> opposite order:
> 
> msi_device_data_release()
> ...
> pcim_release()
>pci_disable_msi[x]()
> 
> which is obviously wrong, because pci_disable_msi[x]() requires the MSI
> data to be available to tear down the MSI[-X] interrupts.
> 
> Remove the MSI[-X] teardown from pcim_release() and add an explicit action
> to be installed on the attempt of enabling PCI/MSI[-X].
> 
> This allows the MSI core data allocation to be ordered correctly in a
> subsequent step.
> 
> Reported-by: Nishanth Menon 
> Signed-off-by: Thomas Gleixner 
> ---
> V4: New patch


Reviewed-by: Greg Kroah-Hartman 

Re: [PATCH v7 2/3] usb: Introduce Xen pvUSB frontend (xen hcd)

[Greg Kroah-Hartman]

On Fri, Dec 03, 2021 at 01:50:44PM +0100, Juergen Gross wrote:
> On 03.12.21 13:49, Greg Kroah-Hartman wrote:
> > On Tue, Nov 23, 2021 at 02:20:47PM +0100, Juergen Gross wrote:
> > > Introduces the Xen pvUSB frontend. With pvUSB it is possible for a Xen
> > > domU to communicate with a USB device assigned to that domU. The
> > > communication is all done via the pvUSB backend in a driver domain
> > > (usually Dom0) which is owner of the physical device.
> > > 
> > > The pvUSB frontend is a USB hcd for a virtual USB host connector.
> > > 
> > > The code is taken from the pvUSB implementation in Xen done by Fujitsu
> > > based on Linux kernel 2.6.18.
> > > 
> > > Changes from the original version are:
> > > - port to upstream kernel
> > > - put all code in just one source file
> > > - move module to appropriate location in kernel tree
> > > - adapt to Linux style guide
> > > - minor code modifications to increase readability
> > > 
> > > Signed-off-by: Juergen Gross 
> > > ---
> > >   drivers/usb/host/Kconfig   |   11 +
> > >   drivers/usb/host/Makefile  |1 +
> > >   drivers/usb/host/xen-hcd.c | 1606 
> > >   3 files changed, 1618 insertions(+)
> > >   create mode 100644 drivers/usb/host/xen-hcd.c
> > 
> > This looks sane to me, but I don't know the HCD interface as well as
> > others on linux-usb do, like Alan Stern.
> > 
> > What tree do you want this to be merged through, my USB one?
> 
> Either that, or I can carry it through the Xen tree.
> 
> Its your choice. :-)

I've grabbed them now, thanks.

greg k-h

Re: [patch V3 03/35] x86/apic/msi: Use PCI device MSI property

[Greg Kroah-Hartman]

On Fri, Dec 10, 2021 at 11:18:47PM +0100, Thomas Gleixner wrote:
> From: Thomas Gleixner 
> 
> instead of fiddling with MSI descriptors.
> 
> Signed-off-by: Thomas Gleixner 

Reviewed-by: Greg Kroah-Hartman 

Re: [patch V2 29/31] genirq/msi: Add abuse prevention comment to msi header

[Greg Kroah-Hartman]

On Mon, Dec 06, 2021 at 11:51:49PM +0100, Thomas Gleixner wrote:
> Signed-off-by: Thomas Gleixner 
> ---
>  include/linux/msi.h |   14 ++
>  1 file changed, 14 insertions(+)
> 
> --- a/include/linux/msi.h
> +++ b/include/linux/msi.h
> @@ -2,6 +2,20 @@
>  #ifndef LINUX_MSI_H
>  #define LINUX_MSI_H
>  
> +/*
> + * This header file contains MSI data structures and functions which are
> + * only relevant for:
> + *   - Interrupt core code
> + *   - PCI/MSI core code
> + *   - MSI interrupt domain implementations
> + *   - IOMMU, low level VFIO, NTB and other justified exceptions
> + * dealing with low level MSI details.
> + *
> + * Regular device drivers have no business with any of these functions and
> + * especially storing MSI descriptor pointers in random code is considered
> + * abuse. The only function which is relevant for drivers is msi_get_virq().
> + */
> +
>  #include 
>  #include 
>  #include 
> 

Ah, to be young and idealistic and hope that kernel developers read
comments in header files :)

You might want to add this to the driver-api kernel doc build?

Anyway, looks good to me:

Reviewed-by: Greg Kroah-Hartman 

Re: [patch V2 22/23] genirq/msi: Handle PCI/MSI allocation fail in core code

[Greg Kroah-Hartman]

On Mon, Dec 06, 2021 at 11:27:59PM +0100, Thomas Gleixner wrote:
> Get rid of yet another irqdomain callback and let the core code return the
> already available information of how many descriptors could be allocated.
> 
> Signed-off-by: Thomas Gleixner 
> Tested-by: Juergen Gross 
> Reviewed-by: Jason Gunthorpe 

Reviewed-by: Greg Kroah-Hartman 

Re: [patch V2 23/23] PCI/MSI: Move descriptor counting on allocation fail to the legacy code

[Greg Kroah-Hartman]

On Mon, Dec 06, 2021 at 11:28:00PM +0100, Thomas Gleixner wrote:
> The irqdomain code already returns the information. Move the loop to the
> legacy code.
> 
> Signed-off-by: Thomas Gleixner 
> Tested-by: Juergen Gross 
> Reviewed-by: Jason Gunthorpe 

Reviewed-by: Greg Kroah-Hartman 

Re: [patch V2 28/36] PCI/MSI: Use __msi_get_virq() in pci_get_vector()

[Greg Kroah-Hartman]

On Mon, Dec 06, 2021 at 11:39:41PM +0100, Thomas Gleixner wrote:
> Use msi_get_vector() and handle the return value to be compatible.
> 
> No functional change intended.
> 
> Signed-off-by: Thomas Gleixner 

Reviewed-by: Greg Kroah-Hartman 

Re: [patch V2 18/36] genirq/msi: Add msi_device_data::properties

[Greg Kroah-Hartman]

On Mon, Dec 06, 2021 at 11:39:25PM +0100, Thomas Gleixner wrote:
> Add a properties field which allows core code to store information for easy
> retrieval in order to replace MSI descriptor fiddling.
> 
> Signed-off-by: Thomas Gleixner 

Reviewed-by: Greg Kroah-Hartman 

Re: [patch V2 19/36] PCI/MSI: Store properties in device::msi::data

[Greg Kroah-Hartman]

On Mon, Dec 06, 2021 at 11:39:26PM +0100, Thomas Gleixner wrote:
> Store the properties which are interesting for various places so the MSI
> descriptor fiddling can be removed.
> 
> Signed-off-by: Thomas Gleixner 

Reviewed-by: Greg Kroah-Hartman 

Re: [patch V2 23/36] powerpc/cell/axon_msi: Use MSI device properties

[Greg Kroah-Hartman]

On Mon, Dec 06, 2021 at 11:39:33PM +0100, Thomas Gleixner wrote:
> instead of fiddling with MSI descriptors.
> 
> Signed-off-by: Thomas Gleixner 

Reviewed-by: Greg Kroah-Hartman 

Re: [patch V2 26/36] powerpc/pseries/msi: Let core code check for contiguous entries

[Greg Kroah-Hartman]

On Mon, Dec 06, 2021 at 11:39:37PM +0100, Thomas Gleixner wrote:
> Set the domain info flag and remove the check.
> 
> Signed-off-by: Thomas Gleixner 

Reviewed-by: Greg Kroah-Hartman 

Re: [patch V2 27/36] genirq/msi: Provide interface to retrieve Linux interrupt number

[Greg Kroah-Hartman]

On Mon, Dec 06, 2021 at 11:39:39PM +0100, Thomas Gleixner wrote:
> This allows drivers to retrieve the Linux interrupt number instead of
> fiddling with MSI descriptors.
> 
> msi_get_virq() returns the Linux interrupt number or 0 in case that there
> is no entry for the given MSI index.
> 
> Signed-off-by: Thomas Gleixner 

Reviewed-by: Greg Kroah-Hartman 

Re: [patch V2 21/23] PCI/MSI: Make pci_msi_domain_check_cap() static

[Greg Kroah-Hartman]

On Mon, Dec 06, 2021 at 11:27:57PM +0100, Thomas Gleixner wrote:
> No users outside of that file.
> 
> Signed-off-by: Thomas Gleixner 
> Tested-by: Juergen Gross 
> Reviewed-by: Jason Gunthorpe 

Reviewed-by: Greg Kroah-Hartman 

Re: [patch V2 20/23] PCI/MSI: Move msi_lock to struct pci_dev

[Greg Kroah-Hartman]

On Mon, Dec 06, 2021 at 11:27:56PM +0100, Thomas Gleixner wrote:
> It's only required for PCI/MSI. So no point in having it in every struct
> device.
> 
> Signed-off-by: Thomas Gleixner 

Yes!!!


Reviewed-by: Greg Kroah-Hartman 

Re: [patch V2 16/23] PCI/MSI: Split out CONFIG_PCI_MSI independent part

[Greg Kroah-Hartman]

On Mon, Dec 06, 2021 at 11:27:49PM +0100, Thomas Gleixner wrote:
> These functions are required even when CONFIG_PCI_MSI is not set. Move them
> to their own file.
> 
> Signed-off-by: Thomas Gleixner 
> Tested-by: Juergen Gross 
> Reviewed-by: Jason Gunthorpe 


Reviewed-by: Greg Kroah-Hartman 

Re: [patch V2 18/23] PCI/MSI: Split out irqdomain code

[Greg Kroah-Hartman]

On Mon, Dec 06, 2021 at 11:27:52PM +0100, Thomas Gleixner wrote:
> Move the irqdomain specific code into it's own file.
> 
> Signed-off-by: Thomas Gleixner 
> Tested-by: Juergen Gross 
> Reviewed-by: Jason Gunthorpe 

Reviewed-by: Greg Kroah-Hartman 

Re: [patch V2 17/23] PCI/MSI: Split out !IRQDOMAIN code

[Greg Kroah-Hartman]

On Mon, Dec 06, 2021 at 11:27:51PM +0100, Thomas Gleixner wrote:
> Split out the non irqdomain code into its own file.
> 
> Signed-off-by: Thomas Gleixner 
> Tested-by: Juergen Gross 
> Reviewed-by: Jason Gunthorpe 

Reviewed-by: Greg Kroah-Hartman 

Re: [patch V2 14/23] PCI/MSI: Make msix_update_entries() smarter

[Greg Kroah-Hartman]

On Mon, Dec 06, 2021 at 11:27:46PM +0100, Thomas Gleixner wrote:
> No need to walk the descriptors and check for each one whether the entries
> pointer function argument is NULL. Do it once.
> 
> Signed-off-by: Thomas Gleixner 
> Tested-by: Juergen Gross 
> Reviewed-by: Jason Gunthorpe 

Reviewed-by: Greg Kroah-Hartman 

Re: [patch V2 10/23] genirq/msi, treewide: Use a named struct for PCI/MSI attributes

[Greg Kroah-Hartman]

On Mon, Dec 06, 2021 at 11:27:39PM +0100, Thomas Gleixner wrote:
> The unnamed struct sucks and is in the way of further cleanups. Stick the
> PCI related MSI data into a real data structure and cleanup all users.
> 
> No functional change.
> 
> Signed-off-by: Thomas Gleixner 
> Tested-by: Juergen Gross 
> Reviewed-by: Jason Gunthorpe 
> Acked-by: Kalle Valo 
> Cc: Greg Kroah-Hartman 
> Cc: sparcli...@vger.kernel.org
> Cc: x...@kernel.org
> Cc: xen-devel@lists.xenproject.org
> Cc: ath...@lists.infradead.org

Reviewed-by: Greg Kroah-Hartman 

Re: [patch V2 08/23] PCI/sysfs: Use pci_irq_vector()

[Greg Kroah-Hartman]

On Mon, Dec 06, 2021 at 11:27:36PM +0100, Thomas Gleixner wrote:
> instead of fiddling with msi descriptors.
> 
> Signed-off-by: Thomas Gleixner 
> Tested-by: Juergen Gross 
> Reviewed-by: Jason Gunthorpe 

Reviewed-by: Greg Kroah-Hartman 

Re: [patch V2 05/23] genirq/msi: Fixup includes

[Greg Kroah-Hartman]

On Mon, Dec 06, 2021 at 11:27:31PM +0100, Thomas Gleixner wrote:
> Remove the kobject.h include from msi.h as it's not required and add a
> sysfs.h include to the core code instead.
> 
> Signed-off-by: Thomas Gleixner 
> Tested-by: Juergen Gross 
> Reviewed-by: Jason Gunthorpe 

Reviewed-by: Greg Kroah-Hartman 

Re: [patch V2 03/23] genirq/msi: Guard sysfs code

[Greg Kroah-Hartman]

On Mon, Dec 06, 2021 at 11:27:28PM +0100, Thomas Gleixner wrote:
> No point in building unused code when CONFIG_SYSFS=n.
> 
> Signed-off-by: Thomas Gleixner 
> Tested-by: Juergen Gross 
> Reviewed-by: Jason Gunthorpe 

Reviewed-by: Greg Kroah-Hartman 

Re: [PATCH v7 2/3] usb: Introduce Xen pvUSB frontend (xen hcd)

[Greg Kroah-Hartman]

On Tue, Nov 23, 2021 at 02:20:47PM +0100, Juergen Gross wrote:
> Introduces the Xen pvUSB frontend. With pvUSB it is possible for a Xen
> domU to communicate with a USB device assigned to that domU. The
> communication is all done via the pvUSB backend in a driver domain
> (usually Dom0) which is owner of the physical device.
> 
> The pvUSB frontend is a USB hcd for a virtual USB host connector.
> 
> The code is taken from the pvUSB implementation in Xen done by Fujitsu
> based on Linux kernel 2.6.18.
> 
> Changes from the original version are:
> - port to upstream kernel
> - put all code in just one source file
> - move module to appropriate location in kernel tree
> - adapt to Linux style guide
> - minor code modifications to increase readability
> 
> Signed-off-by: Juergen Gross 
> ---
>  drivers/usb/host/Kconfig   |   11 +
>  drivers/usb/host/Makefile  |1 +
>  drivers/usb/host/xen-hcd.c | 1606 
>  3 files changed, 1618 insertions(+)
>  create mode 100644 drivers/usb/host/xen-hcd.c

This looks sane to me, but I don't know the HCD interface as well as
others on linux-usb do, like Alan Stern.

What tree do you want this to be merged through, my USB one?

thanks,

greg k-h

Re: Stable backports of Xen related patches

[Greg Kroah-Hartman]

On Mon, Nov 29, 2021 at 04:15:31PM +0100, Juergen Gross wrote:
> Hi Greg,
> 
> attached are git bundles for some patches you merged into the 5.10
> stable kernel already this morning.
> 
> Naming should be obvious, the patches are on the branch "back" in
> each bundle.

All now queued up, thanks!

greg k-h

Re: Patches for stable 5.10 kernel

[Greg Kroah-Hartman]

On Mon, Nov 29, 2021 at 08:19:18AM +0100, Juergen Gross wrote:
> Hi Greg,
> 
> could you please add the following upstream patches to the stable 5.10
> kernel (I'll send separate mails for the older stable kernels as some
> of the patches don't apply for those)? They are hardening Xen PV
> frontends against attacks from related backends.
> 
> Qubes-OS has asked for those patches to be added to stable, too.
> 
> 629a5d87e26fe96b ("xen: sync include/xen/interface/io/ring.h with Xen's
> newest version")
> 71b66243f9898d0e ("xen/blkfront: read response from backend only once")
> 8f5a695d99000fc3 ("xen/blkfront: don't take local copy of a request from the
> ring page")
> b94e4b147fd1992a ("xen/blkfront: don't trust the backend response data
> blindly")
> 8446066bf8c1f9f7 ("xen/netfront: read response from backend only once")
> 162081ec33c2686a ("xen/netfront: don't read data from request on the ring
> page")
> 21631d2d741a64a0 ("xen/netfront: disentangle tx_skb_freelist")
> a884daa61a7d9165 ("xen/netfront: don't trust the backend response data
> blindly")
> e679004dec37566f ("tty: hvc: replace BUG_ON() with negative return value")
> 

All now queued up, thanks.

But people should be moving to the 5.15 kernel by now and not sticking
with 5.10 anymore for stuff like this.

greg k-h

Re: [PATCH v5 0/3] xen, usb: support pvUSB frontend driver

[Greg Kroah-Hartman]

On Fri, Oct 01, 2021 at 05:00:36PM +0200, Juergen Gross wrote:
> This series adds XEN guest pvUSB support. With pvUSB it is possible to
> use physical USB devices from a XEN domain.
> 
> Since V4 a lot of time (several years) has passed. This is a revived
> attempt to get the frontend into the kernel.
> 
> The support consists of a frontend in form of a virtual hcd driver in
> the unprivileged domU passing I/O-requests to the backend in a driver
> domain (usually Dom0). The backend is not part of this patch series,
> as it is supported via qemu.
> 
> The code is taken (and adapted) from the original pvUSB implementation
> done for Linux 2.6 in 2008 by Fujitsu.
> 
> Normal operation of USB devices by adding and removing them dynamically
> to/from a domain has been tested using various USB devices (USB 1.1,
> 2.0 and 3.0) using the qemu based backend.
> 
> Changes in V5:
> - added interface documentation to patch 1
> - frontend no longer trusts backend to return only sane values

You forgot to send patch 3/3 to the linux-usb list :(

Re: [PATCH v5 2/3] usb: Introduce Xen pvUSB frontend (xen hcd)

[Greg Kroah-Hartman]

On Fri, Oct 01, 2021 at 05:00:38PM +0200, Juergen Gross wrote:
> --- /dev/null
> +++ b/drivers/usb/host/xen-hcd.c
> @@ -0,0 +1,1641 @@
> +/*
> + * xen-hcd.c

Need a SPDX line here, instead of the license boiler-plate text you
have.

Did you run checkpatch on this thing?  I thought that would have told
you about this.

> + *
> + * Xen USB Virtual Host Controller driver
> + *
> + * Copyright (C) 2009, FUJITSU LABORATORIES LTD.
> + * Author: Noboru Iwamatsu 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, see .
> + *
> + * Or, by your choice:
> + *
> + * When distributed separately from the Linux kernel or incorporated into
> + * other software packages, subject to the following license:
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a 
> copy
> + * of this software and associated documentation files (the "Software"), to
> + * deal in the Software without restriction, including without limitation the
> + * rights to use, copy, modify, merge, publish, distribute, sublicense, 
> and/or
> + * sell copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL 
> THE
> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.

Dual licensing a USB host controller driver is, um, a very dubious
thing given that you can only interact with symbols exported with
EXPORT_SYMBOL_GPL() from the USB stack.

So you might want to change this...

If not, and you want to keep the dual license, I need some explaination
from a legal person at your company included in the changelog text, and
a signed-off-by from them as well.

thanks,

greg k-h

Re: [PATCH 1/2] PM: base: power: don't try to use non-existing RTC for storing data

[Greg Kroah-Hartman]

On Fri, Sep 03, 2021 at 11:01:58AM +0200, Juergen Gross wrote:
> On 03.09.21 10:56, Greg Kroah-Hartman wrote:
> > On Fri, Sep 03, 2021 at 10:49:36AM +0200, Juergen Gross wrote:
> > > In there is no legacy RTC device, don't try to use it for storing trace
> > > data across suspend/resume.
> > > 
> > > Cc: 
> > > Signed-off-by: Juergen Gross 
> > > ---
> > >   drivers/base/power/trace.c | 10 ++
> > >   1 file changed, 10 insertions(+)
> > > 
> > > diff --git a/drivers/base/power/trace.c b/drivers/base/power/trace.c
> > > index a97f33d0c59f..b7c80849455c 100644
> > > --- a/drivers/base/power/trace.c
> > > +++ b/drivers/base/power/trace.c
> > > @@ -13,6 +13,7 @@
> > >   #include 
> > >   #include 
> > >   #include 
> > > +#include 
> > >   #include 
> > > @@ -165,6 +166,9 @@ void generate_pm_trace(const void *tracedata, 
> > > unsigned int user)
> > >   const char *file = *(const char **)(tracedata + 2);
> > >   unsigned int user_hash_value, file_hash_value;
> > > + if (!x86_platform.legacy.rtc)
> > > + return 0;
> > 
> > Why does the driver core code here care about a platform/arch-specific
> > thing at all?  Did you just break all other arches?
> 
> This file is only compiled for x86. It depends on CONFIG_PM_TRACE_RTC,
> which has a "depends on X86" attribute.

Odd, and not obvious at all :(

Ok, I'll let Rafael review this...

Re: [PATCH 1/2] PM: base: power: don't try to use non-existing RTC for storing data

[Greg Kroah-Hartman]

On Fri, Sep 03, 2021 at 10:49:36AM +0200, Juergen Gross wrote:
> In there is no legacy RTC device, don't try to use it for storing trace
> data across suspend/resume.
> 
> Cc: 
> Signed-off-by: Juergen Gross 
> ---
>  drivers/base/power/trace.c | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/base/power/trace.c b/drivers/base/power/trace.c
> index a97f33d0c59f..b7c80849455c 100644
> --- a/drivers/base/power/trace.c
> +++ b/drivers/base/power/trace.c
> @@ -13,6 +13,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  
> @@ -165,6 +166,9 @@ void generate_pm_trace(const void *tracedata, unsigned 
> int user)
>   const char *file = *(const char **)(tracedata + 2);
>   unsigned int user_hash_value, file_hash_value;
>  
> + if (!x86_platform.legacy.rtc)
> + return 0;

Why does the driver core code here care about a platform/arch-specific
thing at all?  Did you just break all other arches?

thanks,

greg k-h

Re: Request for adding patch to stable-5.13

[Greg Kroah-Hartman]

On Mon, Aug 16, 2021 at 01:09:52PM +0200, Juergen Gross wrote:
> Greg,
> 
> could you please add upstream commit 05d69d950d9d8 ("xen-blkfront:
> sanitize the removal state machine") to the stable 5.13 kernel?
> 
> The patch seems not to have a "Cc: stable" tag while it fixes an
> issue introduced in 5.13.

Please always cc: sta...@vger.kernel.org with these requests in the
future...

Anyway, the commit does not cleanly apply, so we will need a working
backport if you want to see it added to that tree.

thanks,

greg k-h

Re: [PATCH v1 0/5] PCI: Drop duplicated tracking of a pci_dev's bound driver

[Greg Kroah-Hartman]

On Thu, Jul 29, 2021 at 10:37:35PM +0200, Uwe Kleine-König wrote:
> Hello,
> 
> struct pci_dev tracks the bound pci driver twice. This series is about
> removing this duplication.
> 
> The first two patches are just cleanups. The third patch introduces a
> wrapper that abstracts access to struct pci_dev->driver. In the next
> patch (hopefully) all users are converted to use the new wrapper and
> finally the fifth patch removes the duplication.
> 
> Note this series is only build tested (allmodconfig on several
> architectures).
> 
> I'm open to restructure this series if this simplifies things. E.g. the
> use of the new wrapper in drivers/pci could be squashed into the patch
> introducing the wrapper. Patch 4 could be split by maintainer tree or
> squashed into patch 3 completely.
> 
> Best regards
> Uwe
> 
> Uwe Kleine-König (5):
>   PCI: Simplify pci_device_remove()
>   PCI: Drop useless check from pci_device_probe()
>   PCI: Provide wrapper to access a pci_dev's bound driver
>   PCI: Adapt all code locations to not use struct pci_dev::driver
> directly
>   PCI: Drop duplicated tracking of a pci_dev's bound driver

Other than my objection to patch 5/5 lack of changelog, looks sane to
me:

Acked-by: Greg Kroah-Hartman 

Re: [PATCH v4 0/5] bus: Make remove callback return void

[Greg Kroah-Hartman]

On Wed, Jul 21, 2021 at 12:09:41PM +0200, Greg Kroah-Hartman wrote:
> On Tue, Jul 13, 2021 at 09:35:17PM +0200, Uwe Kleine-König wrote:
> > Hello,
> > 
> > this is v4 of the final patch set for my effort to make struct
> > bus_type::remove return void.
> > 
> > The first four patches contain cleanups that make some of these
> > callbacks (more obviously) always return 0. They are acked by the
> > respective maintainers. Bjorn Helgaas explicitly asked to include the
> > pci patch (#1) into this series, so Greg taking this is fine. I assume
> > the s390 people are fine with Greg taking patches #2 to #4, too, they
> > didn't explicitly said so though.
> > 
> > The last patch actually changes the prototype and so touches quite some
> > drivers and has the potential to conflict with future developments, so I
> > consider it beneficial to put these patches into next soon. I expect
> > that it will be Greg who takes the complete series, he already confirmed
> > via irc (for v2) to look into this series.
> > 
> > The only change compared to v3 is in the fourth patch where I modified a
> > few more drivers to fix build failures. Some of them were found by build
> > bots (thanks!), some of them I found myself using a regular expression
> > search. The newly modified files are:
> > 
> >  arch/sparc/kernel/vio.c
> >  drivers/nubus/bus.c
> >  drivers/sh/superhyway/superhyway.c
> >  drivers/vlynq/vlynq.c
> >  drivers/zorro/zorro-driver.c
> >  sound/ac97/bus.c
> > 
> > Best regards
> > Uwe
> 
> Now queued up.  I can go make a git tag that people can pull from after
> 0-day is finished testing this to verify all is good, if others need it.

Ok, here's a tag that any other subsystem can pull from if they want
these changes in their tree before 5.15-rc1 is out.  I might pull it
into my char-misc-next tree as well just to keep that tree sane as it
seems to pick up new busses on a regular basis...

thanks,

greg k-h

---


The following changes since commit 2734d6c1b1a089fb593ef6a23d4b70903526fe0c:

  Linux 5.14-rc2 (2021-07-18 14:13:49 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git 
tags/bus_remove_return_void-5.15

for you to fetch changes up to fc7a6209d5710618eb4f72a77cd81b8d694ecf89:

  bus: Make remove callback return void (2021-07-21 11:53:42 +0200)

--------
Bus: Make remove callback return void tag

Tag for other trees/branches to pull from in order to have a stable
place to build off of if they want to add new busses for 5.15.

Signed-off-by: Greg Kroah-Hartman 


Uwe Kleine-König (5):
  PCI: endpoint: Make struct pci_epf_driver::remove return void
  s390/cio: Make struct css_driver::remove return void
  s390/ccwgroup: Drop if with an always false condition
  s390/scm: Make struct scm_driver::remove return void
  bus: Make remove callback return void

 arch/arm/common/locomo.c  | 3 +--
 arch/arm/common/sa.c  | 4 +---
 arch/arm/mach-rpc/ecard.c | 4 +---
 arch/mips/sgi-ip22/ip22-gio.c | 3 +--
 arch/parisc/kernel/drivers.c  | 5 ++---
 arch/powerpc/platforms/ps3/system-bus.c   | 3 +--
 arch/powerpc/platforms/pseries/ibmebus.c  | 3 +--
 arch/powerpc/platforms/pseries/vio.c  | 3 +--
 arch/s390/include/asm/eadm.h  | 2 +-
 arch/sparc/kernel/vio.c   | 4 +---
 drivers/acpi/bus.c| 3 +--
 drivers/amba/bus.c| 4 +---
 drivers/base/auxiliary.c  | 4 +---
 drivers/base/isa.c| 4 +---
 drivers/base/platform.c   | 4 +---
 drivers/bcma/main.c   | 6 ++
 drivers/bus/sunxi-rsb.c   | 4 +---
 drivers/cxl/core.c| 3 +--
 drivers/dax/bus.c | 4 +---
 drivers/dma/idxd/sysfs.c  | 4 +---
 drivers/firewire/core-device.c| 4 +---
 drivers/firmware/arm_scmi/bus.c   | 4 +---
 drivers/firmware/google/coreboot_table.c  | 4 +---
 drivers/fpga/dfl.c| 4 +---
 drivers/hid/hid-core.c| 4 +---
 drivers/hid/intel-ish-hid/ishtp/bus.c | 4 +---
 drivers/hv/vmbus_drv.c| 5 +
 drivers/hwtracing/intel_th/core.c | 4 +---
 drivers/i2c/i2c-core-base.c   | 5 +
 drivers/i3c/master.c  | 4 +---
 drivers/input/gameport/gameport.c | 3 +--
 drivers/input/serio/serio.c   | 3 +--
 drivers/ipack/ipack.c | 4 +---
 drivers/macint

Re: [PATCH v4 0/5] bus: Make remove callback return void

[Greg Kroah-Hartman]

On Tue, Jul 13, 2021 at 09:35:17PM +0200, Uwe Kleine-König wrote:
> Hello,
> 
> this is v4 of the final patch set for my effort to make struct
> bus_type::remove return void.
> 
> The first four patches contain cleanups that make some of these
> callbacks (more obviously) always return 0. They are acked by the
> respective maintainers. Bjorn Helgaas explicitly asked to include the
> pci patch (#1) into this series, so Greg taking this is fine. I assume
> the s390 people are fine with Greg taking patches #2 to #4, too, they
> didn't explicitly said so though.
> 
> The last patch actually changes the prototype and so touches quite some
> drivers and has the potential to conflict with future developments, so I
> consider it beneficial to put these patches into next soon. I expect
> that it will be Greg who takes the complete series, he already confirmed
> via irc (for v2) to look into this series.
> 
> The only change compared to v3 is in the fourth patch where I modified a
> few more drivers to fix build failures. Some of them were found by build
> bots (thanks!), some of them I found myself using a regular expression
> search. The newly modified files are:
> 
>  arch/sparc/kernel/vio.c
>  drivers/nubus/bus.c
>  drivers/sh/superhyway/superhyway.c
>  drivers/vlynq/vlynq.c
>  drivers/zorro/zorro-driver.c
>  sound/ac97/bus.c
> 
> Best regards
> Uwe

Now queued up.  I can go make a git tag that people can pull from after
0-day is finished testing this to verify all is good, if others need it.

thanks,

greg k-h

Re: Backport of commit 3de218ff39b9e3f0d4

[Greg Kroah-Hartman]

On Mon, Jul 05, 2021 at 01:58:09PM +0200, Juergen Gross wrote:
> Hi Greg,
> 
> the attached patch is a backport of upstream commit 3de218ff39b9e3f0d4
> for Linux 5.10 and older (I've checked it to apply down to 4.4).

Now applied, thanks.

greg k-h

Re: [PATCH v2 0/4] Support xen-driven USB3 debug capability

[Greg Kroah-Hartman]

On Thu, May 13, 2021 at 06:56:47PM -0600, Connor Davis wrote:
> Hi all,
> 
> This goal of this series is to allow the USB3 debug capability (DbC) to be
> safely used by xen while linux runs as dom0.

Patch 2/4 does not seem to be showing up anywhere, did it get lost?

thanks,

greg k-h

Re: [PATCH 8/8] xen/hvc: replace BUG_ON() with negative return value

[Greg Kroah-Hartman]

On Thu, May 13, 2021 at 12:03:02PM +0200, Juergen Gross wrote:
> Xen frontends shouldn't BUG() in case of illegal data received from
> their backends. So replace the BUG_ON()s when reading illegal data from
> the ring page with negative return values.
> 
> Signed-off-by: Juergen Gross 
> ---
>  drivers/tty/hvc/hvc_xen.c | 15 +--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
> index 92c9a476defc..30d7ffb1e04c 100644
> --- a/drivers/tty/hvc/hvc_xen.c
> +++ b/drivers/tty/hvc/hvc_xen.c
> @@ -86,6 +86,11 @@ static int __write_console(struct xencons_info *xencons,
>   cons = intf->out_cons;
>   prod = intf->out_prod;
>   mb();   /* update queue values before going on */
> +
> + if (WARN_ONCE((prod - cons) > sizeof(intf->out),
> +   "Illegal ring page indices"))
> + return -EINVAL;

How nice, you just rebooted on panic-on-warn systems :(

> +
>   BUG_ON((prod - cons) > sizeof(intf->out));

Why keep this line?

Please just fix this up properly, if userspace can trigger this, then
both the WARN_ON() and BUG_ON() are not correct and need to be correctly
handled.


>  
>   while ((sent < len) && ((prod - cons) < sizeof(intf->out)))
> @@ -114,7 +119,10 @@ static int domU_write_console(uint32_t vtermno, const 
> char *data, int len)
>*/
>   while (len) {
>   int sent = __write_console(cons, data, len);
> - 
> +
> + if (sent < 0)
> + return sent;
> +
>   data += sent;
>   len -= sent;
>  
> @@ -138,7 +146,10 @@ static int domU_read_console(uint32_t vtermno, char 
> *buf, int len)
>   cons = intf->in_cons;
>   prod = intf->in_prod;
>   mb();   /* get pointers before reading ring */
> - BUG_ON((prod - cons) > sizeof(intf->in));
> +
> + if (WARN_ONCE((prod - cons) > sizeof(intf->in),
> +   "Illegal ring page indices"))
> + return -EINVAL;

Same here, you still just paniced a machine :(

thanks,

greg k-h

Re: [PATCH 3/3] usb: xhci: Notify xen when DbC is unsafe to use

[Greg Kroah-Hartman]

On Tue, May 11, 2021 at 06:18:21PM -0600, Connor Davis wrote:
> When running as a dom0 guest on Xen, check if the USB3 debug
> capability is enabled before xHCI reset, suspend, and resume. If it
> is, call xen_dbgp_reset_prep() to notify Xen that it is unsafe to touch
> MMIO registers until the next xen_dbgp_external_startup().
> 
> This notification allows Xen to avoid undefined behavior resulting
> from MMIO access when the host controller's CNR bit is set or when
> the device transitions to D3hot.
> 
> Signed-off-by: Connor Davis 
> ---
>  drivers/usb/host/xhci-dbgcap.h |  6 
>  drivers/usb/host/xhci.c| 57 ++
>  drivers/usb/host/xhci.h|  1 +
>  3 files changed, 64 insertions(+)
> 
> diff --git a/drivers/usb/host/xhci-dbgcap.h b/drivers/usb/host/xhci-dbgcap.h
> index c70b78d504eb..24784b82a840 100644
> --- a/drivers/usb/host/xhci-dbgcap.h
> +++ b/drivers/usb/host/xhci-dbgcap.h
> @@ -227,4 +227,10 @@ static inline int xhci_dbc_resume(struct xhci_hcd *xhci)
>   return 0;
>  }
>  #endif /* CONFIG_USB_XHCI_DBGCAP */
> +
> +#ifdef CONFIG_XEN_DOM0
> +int xen_dbgp_reset_prep(struct usb_hcd *hcd);
> +int xen_dbgp_external_startup(struct usb_hcd *hcd);
> +#endif /* CONFIG_XEN_DOM0 */
> +
>  #endif /* __LINUX_XHCI_DBGCAP_H */
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index ca9385d22f68..afe44169183f 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -37,6 +37,57 @@ static unsigned long long quirks;
>  module_param(quirks, ullong, S_IRUGO);
>  MODULE_PARM_DESC(quirks, "Bit flags for quirks to be enabled as default");
> 
> +#ifdef CONFIG_XEN_DOM0
> +#include 



Can't this #ifdef stuff go into a .h file?

thanks,

greg k-h

Re: [PATCH for-4.19.y] xen/privcmd: allow fetching resource sizes

[Greg Kroah-Hartman]

On Fri, Jan 29, 2021 at 01:22:15PM +0100, Roger Pau Monne wrote:
> commit ef3a575baf53571dc405ee4028e26f50856898e7 upstream.
> 

Now queued up, thanks.

greg k-h

[PATCH 5.4 01/62] x86/asm/32: Add ENDs to some functions and relabel with SYM_CODE_*

[Greg Kroah-Hartman]

From: Jiri Slaby 

commit 78762b0e79bc1dd01347be061abdf505202152c9 upstream.

All these are functions which are invoked from elsewhere but they are
not typical C functions. So annotate them using the new SYM_CODE_START.
All these were not balanced with any END, so mark their ends by
SYM_CODE_END, appropriately.

Signed-off-by: Jiri Slaby 
Signed-off-by: Borislav Petkov 
Reviewed-by: Boris Ostrovsky  [xen bits]
Reviewed-by: Rafael J. Wysocki  [hibernate]
Cc: Andy Lutomirski 
Cc: "H. Peter Anvin" 
Cc: Ingo Molnar 
Cc: Josh Poimboeuf 
Cc: Juergen Gross 
Cc: Len Brown 
Cc: linux-a...@vger.kernel.org
Cc: linux...@vger.kernel.org
Cc: Pavel Machek 
Cc: Peter Zijlstra 
Cc: Pingfan Liu 
Cc: Stefano Stabellini 
Cc: "Steven Rostedt (VMware)" 
Cc: Thomas Gleixner 
Cc: x86-ml 
Cc: xen-devel@lists.xenproject.org
Link: https://lkml.kernel.org/r/2019105108.12392-26-jsl...@suse.cz
Signed-off-by: Sasha Levin 
---
 arch/x86/entry/entry_32.S| 3 ++-
 arch/x86/kernel/acpi/wakeup_32.S | 7 ---
 arch/x86/kernel/ftrace_32.S  | 3 ++-
 arch/x86/kernel/head_32.S| 3 ++-
 arch/x86/power/hibernate_asm_32.S| 6 --
 arch/x86/realmode/rm/trampoline_32.S | 6 --
 arch/x86/xen/xen-asm_32.S| 7 ---
 7 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S
index 390edb7638265..bde3e0f85425f 100644
--- a/arch/x86/entry/entry_32.S
+++ b/arch/x86/entry/entry_32.S
@@ -869,9 +869,10 @@ GLOBAL(__begin_SYSENTER_singlestep_region)
  * Xen doesn't set %esp to be precisely what the normal SYSENTER
  * entry point expects, so fix it up before using the normal path.
  */
-ENTRY(xen_sysenter_target)
+SYM_CODE_START(xen_sysenter_target)
addl$5*4, %esp  /* remove xen-provided frame */
jmp .Lsysenter_past_esp
+SYM_CODE_END(xen_sysenter_target)
 #endif
 
 /*
diff --git a/arch/x86/kernel/acpi/wakeup_32.S b/arch/x86/kernel/acpi/wakeup_32.S
index e95e95960156b..5b076cb79f5fb 100644
--- a/arch/x86/kernel/acpi/wakeup_32.S
+++ b/arch/x86/kernel/acpi/wakeup_32.S
@@ -9,8 +9,7 @@
.code32
ALIGN
 
-ENTRY(wakeup_pmode_return)
-wakeup_pmode_return:
+SYM_CODE_START(wakeup_pmode_return)
movw$__KERNEL_DS, %ax
movw%ax, %ss
movw%ax, %fs
@@ -39,6 +38,7 @@ wakeup_pmode_return:
# jump to place where we left off
movlsaved_eip, %eax
jmp *%eax
+SYM_CODE_END(wakeup_pmode_return)
 
 bogus_magic:
jmp bogus_magic
@@ -72,7 +72,7 @@ restore_registers:
popfl
ret
 
-ENTRY(do_suspend_lowlevel)
+SYM_CODE_START(do_suspend_lowlevel)
callsave_processor_state
callsave_registers
pushl   $3
@@ -87,6 +87,7 @@ ret_point:
callrestore_registers
callrestore_processor_state
ret
+SYM_CODE_END(do_suspend_lowlevel)
 
 .data
 ALIGN
diff --git a/arch/x86/kernel/ftrace_32.S b/arch/x86/kernel/ftrace_32.S
index 073aab525d800..2cc0303522c99 100644
--- a/arch/x86/kernel/ftrace_32.S
+++ b/arch/x86/kernel/ftrace_32.S
@@ -89,7 +89,7 @@ WEAK(ftrace_stub)
ret
 END(ftrace_caller)
 
-ENTRY(ftrace_regs_caller)
+SYM_CODE_START(ftrace_regs_caller)
/*
 * We're here from an mcount/fentry CALL, and the stack frame looks 
like:
 *
@@ -163,6 +163,7 @@ GLOBAL(ftrace_regs_call)
popl%eax
 
jmp .Lftrace_ret
+SYM_CODE_END(ftrace_regs_caller)
 
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
 ENTRY(ftrace_graph_caller)
diff --git a/arch/x86/kernel/head_32.S b/arch/x86/kernel/head_32.S
index 2e6a0676c1f43..11a5d5ade52ce 100644
--- a/arch/x86/kernel/head_32.S
+++ b/arch/x86/kernel/head_32.S
@@ -64,7 +64,7 @@ RESERVE_BRK(pagetables, INIT_MAP_SIZE)
  * can.
  */
 __HEAD
-ENTRY(startup_32)
+SYM_CODE_START(startup_32)
movl pa(initial_stack),%ecx

/* test KEEP_SEGMENTS flag to see if the bootloader is asking
@@ -172,6 +172,7 @@ num_subarch_entries = (. - subarch_entries) / 4
 #else
jmp .Ldefault_entry
 #endif /* CONFIG_PARAVIRT */
+SYM_CODE_END(startup_32)
 
 #ifdef CONFIG_HOTPLUG_CPU
 /*
diff --git a/arch/x86/power/hibernate_asm_32.S 
b/arch/x86/power/hibernate_asm_32.S
index 6fe383002125f..a19ed3d231853 100644
--- a/arch/x86/power/hibernate_asm_32.S
+++ b/arch/x86/power/hibernate_asm_32.S
@@ -35,7 +35,7 @@ ENTRY(swsusp_arch_suspend)
ret
 ENDPROC(swsusp_arch_suspend)
 
-ENTRY(restore_image)
+SYM_CODE_START(restore_image)
/* prepare to jump to the image kernel */
movlrestore_jump_address, %ebx
movlrestore_cr3, %ebp
@@ -45,9 +45,10 @@ ENTRY(restore_image)
/* jump to relocated restore code */
movlrelocated_restore_code, %eax
jmpl*%eax
+SYM_CODE_END(restore_image)
 
 /* code below has been relocated to a safe page */
-ENTRY(core_restore_code)
+SYM_CODE_START(core_restore_code)
movltemp_pgt, %eax
movl%eax, %cr3
 
@@ -77,6 +78,7 @@ co

[PATCH 5.4 05/85] linkage: Introduce new macros for assembler symbols

[Greg Kroah-Hartman]

From: Jiri Slaby 

commit ffedeeb780dc554eff3d3b16e6a462a26a41d7ec upstream.

Introduce new C macros for annotations of functions and data in
assembly. There is a long-standing mess in macros like ENTRY, END,
ENDPROC and similar. They are used in different manners and sometimes
incorrectly.

So introduce macros with clear use to annotate assembly as follows:

a) Support macros for the ones below
   SYM_T_FUNC -- type used by assembler to mark functions
   SYM_T_OBJECT -- type used by assembler to mark data
   SYM_T_NONE -- type used by assembler to mark entries of unknown type

   They are defined as STT_FUNC, STT_OBJECT, and STT_NOTYPE
   respectively. According to the gas manual, this is the most portable
   way. I am not sure about other assemblers, so this can be switched
   back to %function and %object if this turns into a problem.
   Architectures can also override them by something like ", @function"
   if they need.

   SYM_A_ALIGN, SYM_A_NONE -- align the symbol?
   SYM_L_GLOBAL, SYM_L_WEAK, SYM_L_LOCAL -- linkage of symbols

b) Mostly internal annotations, used by the ones below
   SYM_ENTRY -- use only if you have to (for non-paired symbols)
   SYM_START -- use only if you have to (for paired symbols)
   SYM_END -- use only if you have to (for paired symbols)

c) Annotations for code
   SYM_INNER_LABEL_ALIGN -- only for labels in the middle of code
   SYM_INNER_LABEL -- only for labels in the middle of code

   SYM_FUNC_START_LOCAL_ALIAS -- use where there are two local names for
one function
   SYM_FUNC_START_ALIAS -- use where there are two global names for one
function
   SYM_FUNC_END_ALIAS -- the end of LOCAL_ALIASed or ALIASed function

   SYM_FUNC_START -- use for global functions
   SYM_FUNC_START_NOALIGN -- use for global functions, w/o alignment
   SYM_FUNC_START_LOCAL -- use for local functions
   SYM_FUNC_START_LOCAL_NOALIGN -- use for local functions, w/o
alignment
   SYM_FUNC_START_WEAK -- use for weak functions
   SYM_FUNC_START_WEAK_NOALIGN -- use for weak functions, w/o alignment
   SYM_FUNC_END -- the end of SYM_FUNC_START_LOCAL, SYM_FUNC_START,
SYM_FUNC_START_WEAK, ...

   For functions with special (non-C) calling conventions:
   SYM_CODE_START -- use for non-C (special) functions
   SYM_CODE_START_NOALIGN -- use for non-C (special) functions, w/o
alignment
   SYM_CODE_START_LOCAL -- use for local non-C (special) functions
   SYM_CODE_START_LOCAL_NOALIGN -- use for local non-C (special)
functions, w/o alignment
   SYM_CODE_END -- the end of SYM_CODE_START_LOCAL or SYM_CODE_START

d) For data
   SYM_DATA_START -- global data symbol
   SYM_DATA_START_LOCAL -- local data symbol
   SYM_DATA_END -- the end of the SYM_DATA_START symbol
   SYM_DATA_END_LABEL -- the labeled end of SYM_DATA_START symbol
   SYM_DATA -- start+end wrapper around simple global data
   SYM_DATA_LOCAL -- start+end wrapper around simple local data

==

The macros allow to pair starts and ends of functions and mark functions
correctly in the output ELF objects.

All users of the old macros in x86 are converted to use these in further
patches.

Signed-off-by: Jiri Slaby 
Signed-off-by: Borislav Petkov 
Acked-by: Rafael J. Wysocki 
Cc: Andrew Morton 
Cc: Andrey Ryabinin 
Cc: Boris Ostrovsky 
Cc: "H. Peter Anvin" 
Cc: Ingo Molnar 
Cc: Jonathan Corbet 
Cc: Josh Poimboeuf 
Cc: Juergen Gross 
Cc: Len Brown 
Cc: Linus Torvalds 
Cc: linux-a...@vger.kernel.org
Cc: linux-...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: linux...@vger.kernel.org
Cc: Mark Rutland 
Cc: Pavel Machek 
Cc: Peter Zijlstra 
Cc: Thomas Gleixner 
Cc: Will Deacon 
Cc: x86-ml 
Cc: xen-devel@lists.xenproject.org
Link: https://lkml.kernel.org/r/2019105108.12392-2-jsl...@suse.cz
Cc: Jian Cai 
Signed-off-by: Greg Kroah-Hartman 

---
 Documentation/asm-annotations.rst |  216 +
 Documentation/index.rst   |8 +
 arch/x86/include/asm/linkage.h|   10 +
 include/linux/linkage.h   |  245 --
 4 files changed, 468 insertions(+), 11 deletions(-)

--- /dev/null
+++ b/Documentation/asm-annotations.rst
@@ -0,0 +1,216 @@
+Assembler Annotations
+=
+
+Copyright (c) 2017-2019 Jiri Slaby
+
+This document describes the new macros for annotation of data and code in
+assembly. In particular, it contains information about ``SYM_FUNC_START``,
+``SYM_FUNC_END``, ``SYM_CODE_START``, and similar.
+
+Rationale
+-
+Some code like entries, trampolines, or boot code needs to be written in
+assembly. The same as in C, such code is grouped into functions and
+accompanied with data. Standard assemblers do not force users into precisely
+marking these pieces as code, data, or even specifying their length.
+Nevertheless, assemblers provide developers with such annotations to aid
+debuggers throughout assembly. On top of that, developers also want to mark
+some functions as *gl

Re: [PATCH v2 20/39] docs: ABI: testing: make the files compatible with ReST output

[Greg Kroah-Hartman]

On Mon, Nov 02, 2020 at 12:04:36PM +0100, Fabrice Gasnier wrote:
> On 10/30/20 11:09 AM, Mauro Carvalho Chehab wrote:
> > Em Fri, 30 Oct 2020 10:19:12 +0100
> > Fabrice Gasnier  escreveu:
> > 
> >> Hi Mauro,
> >>
> >> [...]
> >>
> >>>  
> >>> +What:
> >>> /sys/bus/iio/devices/iio:deviceX/in_count_quadrature_mode_available
> >>> +KernelVersion:   4.12
> >>> +Contact: benjamin.gaign...@st.com
> >>> +Description:
> >>> + Reading returns the list possible quadrature modes.
> >>> +
> >>> +What:
> >>> /sys/bus/iio/devices/iio:deviceX/in_count0_quadrature_mode
> >>> +KernelVersion:   4.12
> >>> +Contact: benjamin.gaign...@st.com
> >>> +Description:
> >>> + Configure the device counter quadrature modes:
> >>> +
> >>> + channel_A:
> >>> + Encoder A input servers as the count input and B as
> >>> + the UP/DOWN direction control input.
> >>> +
> >>> + channel_B:
> >>> + Encoder B input serves as the count input and A as
> >>> + the UP/DOWN direction control input.
> >>> +
> >>> + quadrature:
> >>> + Encoder A and B inputs are mixed to get direction
> >>> + and count with a scale of 0.25.
> >>> +  
> >>
> > 
> > Hi Fabrice,
> > 
> >> I just noticed that since Jonathan question in v1.
> >>
> >> Above ABI has been moved in the past as discussed in [1]. You can take a
> >> look at:
> >> b299d00 IIO: stm32: Remove quadrature related functions from trigger driver
> >>
> >> Could you please remove the above chunk ?
> >>
> >> With that, for the stm32 part:
> >> Acked-by: Fabrice Gasnier 
> > 
> > 
> > Hmm... probably those were re-introduced due to a rebase. This
> > series were originally written about 1,5 years ago.
> > 
> > I'll drop those hunks.
> 
> Hi Mauro, Greg,
> 
> I just figured out this patch has been applied with above hunk.
> 
> This should be dropped: is there a fix on its way already ?
> (I may have missed it)

Can you send a fix for just this hunk?

thanks,

greg k-h

Re: [PATCH v2 3/7] mm/memory_hotplug: prepare passing flags to add_memory() and friends

[Greg Kroah-Hartman]

On Tue, Sep 08, 2020 at 10:10:08PM +0200, David Hildenbrand wrote:
> We soon want to pass flags, e.g., to mark added System RAM resources.
> mergeable. Prepare for that.

What are these random "flags", and how do we know what should be passed
to them?

Why not make this an enumerated type so that we know it all works
properly, like the GPF_* flags are?  Passing around a random unsigned
long feels very odd/broken...

thanks,

greg k-h

Re: [PATCH v2 2/7] kernel/resource: move and rename IORESOURCE_MEM_DRIVER_MANAGED

[Greg Kroah-Hartman]

On Tue, Sep 08, 2020 at 10:10:07PM +0200, David Hildenbrand wrote:
> IORESOURCE_MEM_DRIVER_MANAGED currently uses an unused PnP bit, which is
> always set to 0 by hardware. This is far from beautiful (and confusing),
> and the bit only applies to SYSRAM. So let's move it out of the
> bus-specific (PnP) defined bits.
> 
> We'll add another SYSRAM specific bit soon. If we ever need more bits for
> other purposes, we can steal some from "desc", or reshuffle/regroup what we
> have.
> 
> Cc: Andrew Morton 
> Cc: Michal Hocko 
> Cc: Dan Williams 
> Cc: Jason Gunthorpe 
> Cc: Kees Cook 
> Cc: Ard Biesheuvel 
> Cc: Pankaj Gupta 
> Cc: Baoquan He 
> Cc: Wei Yang 
> Cc: Eric Biederman 
> Cc: Thomas Gleixner 
> Cc: Greg Kroah-Hartman 
> Cc: ke...@lists.infradead.org
> Signed-off-by: David Hildenbrand 
> ---
>  include/linux/ioport.h | 4 +++-
>  kernel/kexec_file.c| 2 +-
>  mm/memory_hotplug.c| 4 ++--
>  3 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/ioport.h b/include/linux/ioport.h
> index 52a91f5fa1a36..d7620d7c941a0 100644
> --- a/include/linux/ioport.h
> +++ b/include/linux/ioport.h
> @@ -58,6 +58,9 @@ struct resource {
>  #define IORESOURCE_EXT_TYPE_BITS 0x0100  /* Resource extended types */
>  #define IORESOURCE_SYSRAM0x0100  /* System RAM (modifier) */
>  
> +/* IORESOURCE_SYSRAM specific bits. */
> +#define IORESOURCE_SYSRAM_DRIVER_MANAGED 0x0200 /* Always detected 
> via a driver. */
> +

Can't you use BIT() here?

thanks,

greg k-h

[PATCH 4.19 57/71] Fix build error when CONFIG_ACPI is not set/enabled:

[Greg Kroah-Hartman]

From: Randy Dunlap 

[ Upstream commit ee87e1557c42dc9c2da11c38e11b87c311569853 ]

../arch/x86/pci/xen.c: In function ‘pci_xen_init’:
../arch/x86/pci/xen.c:410:2: error: implicit declaration of function 
‘acpi_noirq_set’; did you mean ‘acpi_irq_get’? 
[-Werror=implicit-function-declaration]
  acpi_noirq_set();

Fixes: 88e9ca161c13 ("xen/pci: Use acpi_noirq_set() helper to avoid #ifdef")
Signed-off-by: Randy Dunlap 
Reviewed-by: Juergen Gross 
Cc: Andy Shevchenko 
Cc: Bjorn Helgaas 
Cc: Konrad Rzeszutek Wilk 
Cc: xen-devel@lists.xenproject.org
Cc: linux-...@vger.kernel.org
Signed-off-by: Juergen Gross 
Signed-off-by: Sasha Levin 
---
 arch/x86/pci/xen.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
index 9112d1cb397bb..22da9bfd8a458 100644
--- a/arch/x86/pci/xen.c
+++ b/arch/x86/pci/xen.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 static int xen_pcifront_enable_irq(struct pci_dev *dev)
-- 
2.25.1

[PATCH 5.4 093/107] Fix build error when CONFIG_ACPI is not set/enabled:

[Greg Kroah-Hartman]

From: Randy Dunlap 

[ Upstream commit ee87e1557c42dc9c2da11c38e11b87c311569853 ]

../arch/x86/pci/xen.c: In function ‘pci_xen_init’:
../arch/x86/pci/xen.c:410:2: error: implicit declaration of function 
‘acpi_noirq_set’; did you mean ‘acpi_irq_get’? 
[-Werror=implicit-function-declaration]
  acpi_noirq_set();

Fixes: 88e9ca161c13 ("xen/pci: Use acpi_noirq_set() helper to avoid #ifdef")
Signed-off-by: Randy Dunlap 
Reviewed-by: Juergen Gross 
Cc: Andy Shevchenko 
Cc: Bjorn Helgaas 
Cc: Konrad Rzeszutek Wilk 
Cc: xen-devel@lists.xenproject.org
Cc: linux-...@vger.kernel.org
Signed-off-by: Juergen Gross 
Signed-off-by: Sasha Levin 
---
 arch/x86/pci/xen.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
index 91220cc258547..5c11ae66b5d8e 100644
--- a/arch/x86/pci/xen.c
+++ b/arch/x86/pci/xen.c
@@ -26,6 +26,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 static int xen_pcifront_enable_irq(struct pci_dev *dev)
-- 
2.25.1

[PATCH 5.7 104/124] Fix build error when CONFIG_ACPI is not set/enabled:

[Greg Kroah-Hartman]

From: Randy Dunlap 

[ Upstream commit ee87e1557c42dc9c2da11c38e11b87c311569853 ]

../arch/x86/pci/xen.c: In function ‘pci_xen_init’:
../arch/x86/pci/xen.c:410:2: error: implicit declaration of function 
‘acpi_noirq_set’; did you mean ‘acpi_irq_get’? 
[-Werror=implicit-function-declaration]
  acpi_noirq_set();

Fixes: 88e9ca161c13 ("xen/pci: Use acpi_noirq_set() helper to avoid #ifdef")
Signed-off-by: Randy Dunlap 
Reviewed-by: Juergen Gross 
Cc: Andy Shevchenko 
Cc: Bjorn Helgaas 
Cc: Konrad Rzeszutek Wilk 
Cc: xen-devel@lists.xenproject.org
Cc: linux-...@vger.kernel.org
Signed-off-by: Juergen Gross 
Signed-off-by: Sasha Levin 
---
 arch/x86/pci/xen.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
index 91220cc258547..5c11ae66b5d8e 100644
--- a/arch/x86/pci/xen.c
+++ b/arch/x86/pci/xen.c
@@ -26,6 +26,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 static int xen_pcifront_enable_irq(struct pci_dev *dev)
-- 
2.25.1

[PATCH 5.8 121/148] Fix build error when CONFIG_ACPI is not set/enabled:

[Greg Kroah-Hartman]

From: Randy Dunlap 

[ Upstream commit ee87e1557c42dc9c2da11c38e11b87c311569853 ]

../arch/x86/pci/xen.c: In function ‘pci_xen_init’:
../arch/x86/pci/xen.c:410:2: error: implicit declaration of function 
‘acpi_noirq_set’; did you mean ‘acpi_irq_get’? 
[-Werror=implicit-function-declaration]
  acpi_noirq_set();

Fixes: 88e9ca161c13 ("xen/pci: Use acpi_noirq_set() helper to avoid #ifdef")
Signed-off-by: Randy Dunlap 
Reviewed-by: Juergen Gross 
Cc: Andy Shevchenko 
Cc: Bjorn Helgaas 
Cc: Konrad Rzeszutek Wilk 
Cc: xen-devel@lists.xenproject.org
Cc: linux-...@vger.kernel.org
Signed-off-by: Juergen Gross 
Signed-off-by: Sasha Levin 
---
 arch/x86/pci/xen.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
index e3f1ca3160684..db34fee931388 100644
--- a/arch/x86/pci/xen.c
+++ b/arch/x86/pci/xen.c
@@ -26,6 +26,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 static int xen_pcifront_enable_irq(struct pci_dev *dev)
-- 
2.25.1

Re: [PATCH] xen: don't reschedule in preemption off sections

[Greg Kroah-Hartman]

On Thu, Aug 20, 2020 at 07:58:55AM +0200, Jürgen Groß wrote:
> On 20.08.20 07:44, Greg Kroah-Hartman wrote:
> > On Thu, Aug 20, 2020 at 07:13:00AM +0200, Jürgen Groß wrote:
> > > On 19.08.20 18:57, Sarah Newman wrote:
> > > > On 7/10/20 5:01 AM, Jürgen Groß wrote:
> > > > > On 10.07.20 13:55, Jan Beulich wrote:
> > > > > > On 10.07.2020 12:50, Jürgen Groß wrote:
> > > > > > > On 10.07.20 11:49, Jan Beulich wrote:
> > > > > > > > On 10.07.2020 09:50, Juergen Gross wrote:
> > > > > > > > > For support of long running hypercalls 
> > > > > > > > > xen_maybe_preempt_hcall() is
> > > > > > > > > calling cond_resched() in case a hypercall marked as 
> > > > > > > > > preemptible has
> > > > > > > > > been interrupted.
> > > > > > > > > 
> > > > > > > > > Normally this is no problem, as only hypercalls done
> > > > > > > > > via some ioctl()s
> > > > > > > > > are marked to be preemptible. In rare cases when during such a
> > > > > > > > > preemptible hypercall an interrupt occurs and any softirq 
> > > > > > > > > action is
> > > > > > > > > started from irq_exit(), a further hypercall issued by the 
> > > > > > > > > softirq
> > > > > > > > > handler will be regarded to be preemptible, too. This might 
> > > > > > > > > lead to
> > > > > > > > > rescheduling in spite of the softirq handler potentially 
> > > > > > > > > having set
> > > > > > > > > preempt_disable(), leading to splats like:
> > > > > > > > > 
> > > > > > > > > BUG: sleeping function called from invalid context
> > > > > > > > > at drivers/xen/preempt.c:37
> > > > > > > > > in_atomic(): 1, irqs_disabled(): 0, non_block: 0,
> > > > > > > > > pid: 20775, name: xl
> > > > > > > > > INFO: lockdep is turned off.
> > > > > > > > > CPU: 1 PID: 20775 Comm: xl Tainted: G D W
> > > > > > > > > 5.4.46-1_prgmr_debug.el7.x86_64 #1
> > > > > > > > > Call Trace:
> > > > > > > > > 
> > > > > > > > > dump_stack+0x8f/0xd0
> > > > > > > > > ___might_sleep.cold.76+0xb2/0x103
> > > > > > > > > xen_maybe_preempt_hcall+0x48/0x70
> > > > > > > > > xen_do_hypervisor_callback+0x37/0x40
> > > > > > > > > RIP: e030:xen_hypercall_xen_version+0xa/0x20
> > > > > > > > > Code: ...
> > > > > > > > > RSP: e02b:c900400dcc30 EFLAGS: 0246
> > > > > > > > > RAX: 0004000d RBX: 0200 RCX: 
> > > > > > > > > 8100122a
> > > > > > > > > RDX: 88812e788000 RSI:  RDI: 
> > > > > > > > > 
> > > > > > > > > RBP: 83ee3ad0 R08: 0001 R09: 
> > > > > > > > > 0001
> > > > > > > > > R10:  R11: 0246 R12: 
> > > > > > > > > 8881824aa0b0
> > > > > > > > > R13: 000865496000 R14: 000865496000 R15: 
> > > > > > > > > 88815d04
> > > > > > > > > ? xen_hypercall_xen_version+0xa/0x20
> > > > > > > > > ? xen_force_evtchn_callback+0x9/0x10
> > > > > > > > > ? check_events+0x12/0x20
> > > > > > > > > ? xen_restore_fl_direct+0x1f/0x20
> > > > > > > > > ? _raw_spin_unlock_irqrestore+0x53/0x60
> > > > > > > > > ? debug_dma_sync_single_for_cpu+0x91/0xc0
> > > > > > > > > ? _raw_spin_unlock_irqrestore+0x53/0x60
> > > > > > > > > ? xen_swiotlb_sync_single_for_cpu+0x3d/0x140
> > > > > > > > > ? mlx4_en_process_rx_cq+0x6b6/0x1110 [mlx4_en]
> > > > > > > > > ? mlx4_en_poll_rx_cq+0x64/0x100 [mlx4_en]
> > > > > > > > > ? net_rx_action+0x151/0x4a0
> > > > > > > 

Re: [PATCH] xen: don't reschedule in preemption off sections

[Greg Kroah-Hartman]

On Thu, Aug 20, 2020 at 07:13:00AM +0200, Jürgen Groß wrote:
> On 19.08.20 18:57, Sarah Newman wrote:
> > On 7/10/20 5:01 AM, Jürgen Groß wrote:
> > > On 10.07.20 13:55, Jan Beulich wrote:
> > > > On 10.07.2020 12:50, Jürgen Groß wrote:
> > > > > On 10.07.20 11:49, Jan Beulich wrote:
> > > > > > On 10.07.2020 09:50, Juergen Gross wrote:
> > > > > > > For support of long running hypercalls xen_maybe_preempt_hcall() 
> > > > > > > is
> > > > > > > calling cond_resched() in case a hypercall marked as preemptible 
> > > > > > > has
> > > > > > > been interrupted.
> > > > > > > 
> > > > > > > Normally this is no problem, as only hypercalls done
> > > > > > > via some ioctl()s
> > > > > > > are marked to be preemptible. In rare cases when during such a
> > > > > > > preemptible hypercall an interrupt occurs and any softirq action 
> > > > > > > is
> > > > > > > started from irq_exit(), a further hypercall issued by the softirq
> > > > > > > handler will be regarded to be preemptible, too. This might lead 
> > > > > > > to
> > > > > > > rescheduling in spite of the softirq handler potentially having 
> > > > > > > set
> > > > > > > preempt_disable(), leading to splats like:
> > > > > > > 
> > > > > > > BUG: sleeping function called from invalid context
> > > > > > > at drivers/xen/preempt.c:37
> > > > > > > in_atomic(): 1, irqs_disabled(): 0, non_block: 0,
> > > > > > > pid: 20775, name: xl
> > > > > > > INFO: lockdep is turned off.
> > > > > > > CPU: 1 PID: 20775 Comm: xl Tainted: G D W
> > > > > > > 5.4.46-1_prgmr_debug.el7.x86_64 #1
> > > > > > > Call Trace:
> > > > > > > 
> > > > > > > dump_stack+0x8f/0xd0
> > > > > > > ___might_sleep.cold.76+0xb2/0x103
> > > > > > > xen_maybe_preempt_hcall+0x48/0x70
> > > > > > > xen_do_hypervisor_callback+0x37/0x40
> > > > > > > RIP: e030:xen_hypercall_xen_version+0xa/0x20
> > > > > > > Code: ...
> > > > > > > RSP: e02b:c900400dcc30 EFLAGS: 0246
> > > > > > > RAX: 0004000d RBX: 0200 RCX: 8100122a
> > > > > > > RDX: 88812e788000 RSI:  RDI: 
> > > > > > > RBP: 83ee3ad0 R08: 0001 R09: 0001
> > > > > > > R10:  R11: 0246 R12: 8881824aa0b0
> > > > > > > R13: 000865496000 R14: 000865496000 R15: 88815d04
> > > > > > > ? xen_hypercall_xen_version+0xa/0x20
> > > > > > > ? xen_force_evtchn_callback+0x9/0x10
> > > > > > > ? check_events+0x12/0x20
> > > > > > > ? xen_restore_fl_direct+0x1f/0x20
> > > > > > > ? _raw_spin_unlock_irqrestore+0x53/0x60
> > > > > > > ? debug_dma_sync_single_for_cpu+0x91/0xc0
> > > > > > > ? _raw_spin_unlock_irqrestore+0x53/0x60
> > > > > > > ? xen_swiotlb_sync_single_for_cpu+0x3d/0x140
> > > > > > > ? mlx4_en_process_rx_cq+0x6b6/0x1110 [mlx4_en]
> > > > > > > ? mlx4_en_poll_rx_cq+0x64/0x100 [mlx4_en]
> > > > > > > ? net_rx_action+0x151/0x4a0
> > > > > > > ? __do_softirq+0xed/0x55b
> > > > > > > ? irq_exit+0xea/0x100
> > > > > > > ? xen_evtchn_do_upcall+0x2c/0x40
> > > > > > > ? xen_do_hypervisor_callback+0x29/0x40
> > > > > > > 
> > > > > > > ? xen_hypercall_domctl+0xa/0x20
> > > > > > > ? xen_hypercall_domctl+0x8/0x20
> > > > > > > ? privcmd_ioctl+0x221/0x990 [xen_privcmd]
> > > > > > > ? do_vfs_ioctl+0xa5/0x6f0
> > > > > > > ? ksys_ioctl+0x60/0x90
> > > > > > > ? trace_hardirqs_off_thunk+0x1a/0x20
> > > > > > > ? __x64_sys_ioctl+0x16/0x20
> > > > > > > ? do_syscall_64+0x62/0x250
> > > > > > > ? entry_SYSCALL_64_after_hwframe+0x49/0xbe
> > > > > > > 
> > > > > > > Fix that by testing preempt_count() before calling cond_resched().
> > > > > > > 
> > > > > > > In kernel 5.8 this can't happen any more due to the
> > > > > > > entry code rework.
> > > > > > > 
> > > > > > > Reported-by: Sarah Newman 
> > > > > > > Fixes: 0fa2f5cb2b0ecd8 ("sched/preempt, xen: Use
> > > > > > > need_resched() instead of should_resched()")
> > > > > > > Cc: Sarah Newman 
> > > > > > > Signed-off-by: Juergen Gross 
> > > > > > > ---
> > > > > > >    drivers/xen/preempt.c | 2 +-
> > > > > > >    1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > > 
> > > > > > > diff --git a/drivers/xen/preempt.c b/drivers/xen/preempt.c
> > > > > > > index 17240c5325a3..6ad87b5c95ed 100644
> > > > > > > --- a/drivers/xen/preempt.c
> > > > > > > +++ b/drivers/xen/preempt.c
> > > > > > > @@ -27,7 +27,7 @@ EXPORT_SYMBOL_GPL(xen_in_preemptible_hcall);
> > > > > > >    asmlinkage __visible void xen_maybe_preempt_hcall(void)
> > > > > > >    {
> > > > > > >    if (unlikely(__this_cpu_read(xen_in_preemptible_hcall)
> > > > > > > - && need_resched())) {
> > > > > > > + && need_resched() && !preempt_count())) {
> > > > > > 
> > > > > > Doesn't this have the at least latent risk of hiding issues in
> > > > > > other call trees (by no longer triggering logging like the one
> > > > > > that has propmted this change)? Wouldn't it be better to save,
> > > > > > clear, and restore the flag in one of xen_evtchn_do_upcall() or
> > > 

[PATCH 4.4 67/86] x86/paravirt: Remove the unused irq_enable_sysexit pv op

[Greg Kroah-Hartman]

From: Boris Ostrovsky 

commit 88c15ec90ff16880efab92b519436ee17b198477 upstream.

As result of commit "x86/xen: Avoid fast syscall path for Xen PV
guests", the irq_enable_sysexit pv op is not called by Xen PV guests
anymore and since they were the only ones who used it we can
safely remove it.

Signed-off-by: Boris Ostrovsky 
Reviewed-by: Borislav Petkov 
Acked-by: Andy Lutomirski 
Cc: Andrew Morton 
Cc: Andy Lutomirski 
Cc: Borislav Petkov 
Cc: Brian Gerst 
Cc: Denys Vlasenko 
Cc: H. Peter Anvin 
Cc: Linus Torvalds 
Cc: Peter Zijlstra 
Cc: Thomas Gleixner 
Cc: david.vra...@citrix.com
Cc: konrad.w...@oracle.com
Cc: virtualizat...@lists.linux-foundation.org
Cc: xen-devel@lists.xenproject.org
Link: 
http://lkml.kernel.org/r/1447970147-1733-3-git-send-email-boris.ostrov...@oracle.com
Signed-off-by: Ingo Molnar 
Signed-off-by: Greg Kroah-Hartman 

---
 arch/x86/entry/entry_32.S |8 ++--
 arch/x86/include/asm/paravirt.h   |7 ---
 arch/x86/include/asm/paravirt_types.h |9 -
 arch/x86/kernel/asm-offsets.c |3 ---
 arch/x86/kernel/paravirt.c|7 ---
 arch/x86/kernel/paravirt_patch_32.c   |2 --
 arch/x86/kernel/paravirt_patch_64.c   |1 -
 arch/x86/xen/enlighten.c  |3 ---
 arch/x86/xen/xen-asm_32.S |   14 --
 arch/x86/xen/xen-ops.h|3 ---
 10 files changed, 2 insertions(+), 55 deletions(-)

--- a/arch/x86/entry/entry_32.S
+++ b/arch/x86/entry/entry_32.S
@@ -331,7 +331,8 @@ sysenter_past_esp:
 * Return back to the vDSO, which will pop ecx and edx.
 * Don't bother with DS and ES (they already contain __USER_DS).
 */
-   ENABLE_INTERRUPTS_SYSEXIT
+   sti
+   sysexit
 
 .pushsection .fixup, "ax"
 2: movl$0, PT_FS(%esp)
@@ -554,11 +555,6 @@ ENTRY(native_iret)
iret
_ASM_EXTABLE(native_iret, iret_exc)
 END(native_iret)
-
-ENTRY(native_irq_enable_sysexit)
-   sti
-   sysexit
-END(native_irq_enable_sysexit)
 #endif
 
 ENTRY(overflow)
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -938,13 +938,6 @@ extern void default_banner(void);
push %ecx; push %edx;   \
call PARA_INDIRECT(pv_cpu_ops+PV_CPU_read_cr0); \
pop %edx; pop %ecx
-
-#define ENABLE_INTERRUPTS_SYSEXIT  \
-   PARA_SITE(PARA_PATCH(pv_cpu_ops, PV_CPU_irq_enable_sysexit),\
- CLBR_NONE,\
- jmp PARA_INDIRECT(pv_cpu_ops+PV_CPU_irq_enable_sysexit))
-
-
 #else  /* !CONFIG_X86_32 */
 
 /*
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -162,15 +162,6 @@ struct pv_cpu_ops {
 
u64 (*read_pmc)(int counter);
 
-#ifdef CONFIG_X86_32
-   /*
-* Atomically enable interrupts and return to userspace.  This
-* is only used in 32-bit kernels.  64-bit kernels use
-* usergs_sysret32 instead.
-*/
-   void (*irq_enable_sysexit)(void);
-#endif
-
/*
 * Switch to usermode gs and return to 64-bit usermode using
 * sysret.  Only used in 64-bit kernels to return to 64-bit
--- a/arch/x86/kernel/asm-offsets.c
+++ b/arch/x86/kernel/asm-offsets.c
@@ -65,9 +65,6 @@ void common(void) {
OFFSET(PV_IRQ_irq_disable, pv_irq_ops, irq_disable);
OFFSET(PV_IRQ_irq_enable, pv_irq_ops, irq_enable);
OFFSET(PV_CPU_iret, pv_cpu_ops, iret);
-#ifdef CONFIG_X86_32
-   OFFSET(PV_CPU_irq_enable_sysexit, pv_cpu_ops, irq_enable_sysexit);
-#endif
OFFSET(PV_CPU_read_cr0, pv_cpu_ops, read_cr0);
OFFSET(PV_MMU_read_cr2, pv_mmu_ops, read_cr2);
 #endif
--- a/arch/x86/kernel/paravirt.c
+++ b/arch/x86/kernel/paravirt.c
@@ -168,9 +168,6 @@ unsigned paravirt_patch_default(u8 type,
ret = paravirt_patch_ident_64(insnbuf, len);
 
else if (type == PARAVIRT_PATCH(pv_cpu_ops.iret) ||
-#ifdef CONFIG_X86_32
-type == PARAVIRT_PATCH(pv_cpu_ops.irq_enable_sysexit) ||
-#endif
 type == PARAVIRT_PATCH(pv_cpu_ops.usergs_sysret32) ||
 type == PARAVIRT_PATCH(pv_cpu_ops.usergs_sysret64))
/* If operation requires a jmp, then jmp */
@@ -226,7 +223,6 @@ static u64 native_steal_clock(int cpu)
 
 /* These are in entry.S */
 extern void native_iret(void);
-extern void native_irq_enable_sysexit(void);
 extern void native_usergs_sysret32(void);
 extern void native_usergs_sysret64(void);
 
@@ -385,9 +381,6 @@ __visible struct pv_cpu_ops pv_cpu_ops =
 
.load_sp0 = native_load_sp0,
 
-#if defined(CONFIG_X86_32)
-   .irq_enable_sysexit = native_irq_enable_sysexit,
-#endif
 #ifdef CONFIG_X86_64
 #ifdef CONFIG_IA32_EMULATION
.usergs_sysret32 = native_usergs_sysret32,
--- a/arch/x86/kernel/paravirt_patch_32.c
+++ b/arch/x86/kernel/paravirt_patch_32.c
@@ -5,7 +5,6 @@ DEF_NATIVE(pv_

[Xen-devel] [PATCH 5.4 259/434] xen/gntdev: Use select for DMA_SHARED_BUFFER

[Greg Kroah-Hartman]

From: Jason Gunthorpe 

[ Upstream commit fa6614d8ef13c63aac52ad7c07c5e69ce4aba3dd ]

DMA_SHARED_BUFFER can not be enabled by the user (it represents a library
set in the kernel). The kconfig convention is to use select for such
symbols so they are turned on implicitly when the user enables a kconfig
that needs them.

Otherwise the XEN_GNTDEV_DMABUF kconfig is overly difficult to enable.

Fixes: 932d6562179e ("xen/gntdev: Add initial support for dma-buf UAPI")
Cc: Oleksandr Andrushchenko 
Cc: Boris Ostrovsky 
Cc: xen-devel@lists.xenproject.org
Cc: Juergen Gross 
Cc: Stefano Stabellini 
Reviewed-by: Juergen Gross 
Reviewed-by: Oleksandr Andrushchenko 
Signed-off-by: Jason Gunthorpe 
Signed-off-by: Juergen Gross 
Signed-off-by: Sasha Levin 
---
 drivers/xen/Kconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
index 79cc75096f42..a50dadd01093 100644
--- a/drivers/xen/Kconfig
+++ b/drivers/xen/Kconfig
@@ -141,7 +141,8 @@ config XEN_GNTDEV
 
 config XEN_GNTDEV_DMABUF
bool "Add support for dma-buf grant access device driver extension"
-   depends on XEN_GNTDEV && XEN_GRANT_DMA_ALLOC && DMA_SHARED_BUFFER
+   depends on XEN_GNTDEV && XEN_GRANT_DMA_ALLOC
+   select DMA_SHARED_BUFFER
help
  Allows userspace processes and kernel modules to use Xen backed
  dma-buf implementation. With this extension grant references to
-- 
2.20.1




___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH 4.19 135/219] xen/gntdev: Use select for DMA_SHARED_BUFFER

[Greg Kroah-Hartman]

From: Jason Gunthorpe 

[ Upstream commit fa6614d8ef13c63aac52ad7c07c5e69ce4aba3dd ]

DMA_SHARED_BUFFER can not be enabled by the user (it represents a library
set in the kernel). The kconfig convention is to use select for such
symbols so they are turned on implicitly when the user enables a kconfig
that needs them.

Otherwise the XEN_GNTDEV_DMABUF kconfig is overly difficult to enable.

Fixes: 932d6562179e ("xen/gntdev: Add initial support for dma-buf UAPI")
Cc: Oleksandr Andrushchenko 
Cc: Boris Ostrovsky 
Cc: xen-devel@lists.xenproject.org
Cc: Juergen Gross 
Cc: Stefano Stabellini 
Reviewed-by: Juergen Gross 
Reviewed-by: Oleksandr Andrushchenko 
Signed-off-by: Jason Gunthorpe 
Signed-off-by: Juergen Gross 
Signed-off-by: Sasha Levin 
---
 drivers/xen/Kconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
index 90d387b50ab7..0505eeb593b5 100644
--- a/drivers/xen/Kconfig
+++ b/drivers/xen/Kconfig
@@ -158,7 +158,8 @@ config XEN_GNTDEV
 
 config XEN_GNTDEV_DMABUF
bool "Add support for dma-buf grant access device driver extension"
-   depends on XEN_GNTDEV && XEN_GRANT_DMA_ALLOC && DMA_SHARED_BUFFER
+   depends on XEN_GNTDEV && XEN_GRANT_DMA_ALLOC
+   select DMA_SHARED_BUFFER
help
  Allows userspace processes and kernel modules to use Xen backed
  dma-buf implementation. With this extension grant references to
-- 
2.20.1




___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH] xen-netback: no need to check return value of debugfs_create functions

[Greg Kroah-Hartman]

When calling debugfs functions, there is no need to ever check the
return value.  The function can work or not, but the code logic should
never do something different based on this.

Cc: Wei Liu 
Cc: Paul Durrant 
Cc: xen-devel@lists.xenproject.org
Cc: net...@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman 
---
 drivers/net/xen-netback/netback.c |  3 --
 drivers/net/xen-netback/xenbus.c  | 46 ---
 2 files changed, 11 insertions(+), 38 deletions(-)

diff --git a/drivers/net/xen-netback/netback.c 
b/drivers/net/xen-netback/netback.c
index 1d9940d4e8c7..e018932abf49 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -1653,9 +1653,6 @@ static int __init netback_init(void)
 
 #ifdef CONFIG_DEBUG_FS
xen_netback_dbg_root = debugfs_create_dir("xen-netback", NULL);
-   if (IS_ERR_OR_NULL(xen_netback_dbg_root))
-   pr_warn("Init of debugfs returned %ld!\n",
-   PTR_ERR(xen_netback_dbg_root));
 #endif /* CONFIG_DEBUG_FS */
 
return 0;
diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
index 41034264bd34..f533b7372d59 100644
--- a/drivers/net/xen-netback/xenbus.c
+++ b/drivers/net/xen-netback/xenbus.c
@@ -170,50 +170,26 @@ DEFINE_SHOW_ATTRIBUTE(xenvif_ctrl);
 
 static void xenvif_debugfs_addif(struct xenvif *vif)
 {
-   struct dentry *pfile;
int i;
 
-   if (IS_ERR_OR_NULL(xen_netback_dbg_root))
-   return;
-
vif->xenvif_dbg_root = debugfs_create_dir(vif->dev->name,
  xen_netback_dbg_root);
-   if (!IS_ERR_OR_NULL(vif->xenvif_dbg_root)) {
-   for (i = 0; i < vif->num_queues; ++i) {
-   char filename[sizeof("io_ring_q") + 4];
-
-   snprintf(filename, sizeof(filename), "io_ring_q%d", i);
-   pfile = debugfs_create_file(filename,
-   0600,
-   vif->xenvif_dbg_root,
-   &vif->queues[i],
-   
&xenvif_dbg_io_ring_ops_fops);
-   if (IS_ERR_OR_NULL(pfile))
-   pr_warn("Creation of io_ring file returned 
%ld!\n",
-   PTR_ERR(pfile));
-   }
+   for (i = 0; i < vif->num_queues; ++i) {
+   char filename[sizeof("io_ring_q") + 4];
 
-   if (vif->ctrl_irq) {
-   pfile = debugfs_create_file("ctrl",
-   0400,
-   vif->xenvif_dbg_root,
-   vif,
-   &xenvif_ctrl_fops);
-   if (IS_ERR_OR_NULL(pfile))
-   pr_warn("Creation of ctrl file returned %ld!\n",
-   PTR_ERR(pfile));
-   }
-   } else
-   netdev_warn(vif->dev,
-   "Creation of vif debugfs dir returned %ld!\n",
-   PTR_ERR(vif->xenvif_dbg_root));
+   snprintf(filename, sizeof(filename), "io_ring_q%d", i);
+   debugfs_create_file(filename, 0600, vif->xenvif_dbg_root,
+   &vif->queues[i],
+   &xenvif_dbg_io_ring_ops_fops);
+   }
+
+   if (vif->ctrl_irq)
+   debugfs_create_file("ctrl", 0400, vif->xenvif_dbg_root, vif,
+   &xenvif_ctrl_fops);
 }
 
 static void xenvif_debugfs_delif(struct xenvif *vif)
 {
-   if (IS_ERR_OR_NULL(xen_netback_dbg_root))
-   return;
-
debugfs_remove_recursive(vif->xenvif_dbg_root);
vif->xenvif_dbg_root = NULL;
 }
-- 
2.22.0


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 15/34] staging/vc04_services: convert put_page() to put_user_page*()

[Greg Kroah-Hartman]

On Thu, Aug 01, 2019 at 07:19:46PM -0700, john.hubb...@gmail.com wrote:
> From: John Hubbard 
> 
> For pages that were retained via get_user_pages*(), release those pages
> via the new put_user_page*() routines, instead of via put_page() or
> release_pages().
> 
> This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
> ("mm: introduce put_user_page*(), placeholder versions").
> 
> Cc: Eric Anholt 
> Cc: Stefan Wahren 
> Cc: Greg Kroah-Hartman 
> Cc: Mihaela Muraru 
> Cc: Suniel Mahesh 
> Cc: Al Viro 
> Cc: Sidong Yang 
> Cc: Kishore KP 
> Cc: linux-rpi-ker...@lists.infradead.org
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: de...@driverdev.osuosl.org
> Signed-off-by: John Hubbard 
> ---
>  .../vc04_services/interface/vchiq_arm/vchiq_2835_arm.c | 10 ++----
>  1 file changed, 2 insertions(+), 8 deletions(-)

Acked-by: Greg Kroah-Hartman 

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 10/34] genwqe: convert put_page() to put_user_page*()

[Greg Kroah-Hartman]

On Thu, Aug 01, 2019 at 07:19:41PM -0700, john.hubb...@gmail.com wrote:
> From: John Hubbard 
> 
> For pages that were retained via get_user_pages*(), release those pages
> via the new put_user_page*() routines, instead of via put_page() or
> release_pages().
> 
> This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
> ("mm: introduce put_user_page*(), placeholder versions").
> 
> This changes the release code slightly, because each page slot in the
> page_list[] array is no longer checked for NULL. However, that check
> was wrong anyway, because the get_user_pages() pattern of usage here
> never allowed for NULL entries within a range of pinned pages.
> 
> Cc: Frank Haverkamp 
> Cc: "Guilherme G. Piccoli" 
> Cc: Arnd Bergmann 
> Cc: Greg Kroah-Hartman 
> Signed-off-by: John Hubbard 
> ---
>  drivers/misc/genwqe/card_utils.c | 17 +++------
>  1 file changed, 3 insertions(+), 14 deletions(-)

Acked-by: Greg Kroah-Hartman 

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH 5.0 252/346] block: pass page to xen_biovec_phys_mergeable

[Greg Kroah-Hartman]

[ Upstream commit 0383ad4374f7ad7edd925a2ee4753035c3f5508a ]

xen_biovec_phys_mergeable() only needs .bv_page of the 2nd bio bvec
for checking if the two bvecs can be merged, so pass page to
xen_biovec_phys_mergeable() directly.

No function change.

Cc: ris Ostrovsky 
Cc: Juergen Gross 
Cc: xen-devel@lists.xenproject.org
Cc: Omar Sandoval 
Cc: Christoph Hellwig 
Reviewed-by: Christoph Hellwig 
Reviewed-by: Boris Ostrovsky 
Signed-off-by: Ming Lei 
Signed-off-by: Jens Axboe 
Signed-off-by: Sasha Levin 
---
 block/blk.h| 2 +-
 drivers/xen/biomerge.c | 5 +++--
 include/xen/xen.h  | 4 +++-
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/block/blk.h b/block/blk.h
index 848278c520306..a57bc90e44bb2 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -75,7 +75,7 @@ static inline bool biovec_phys_mergeable(struct request_queue 
*q,
 
if (addr1 + vec1->bv_len != addr2)
return false;
-   if (xen_domain() && !xen_biovec_phys_mergeable(vec1, vec2))
+   if (xen_domain() && !xen_biovec_phys_mergeable(vec1, vec2->bv_page))
return false;
if ((addr1 | mask) != ((addr2 + vec2->bv_len - 1) | mask))
return false;
diff --git a/drivers/xen/biomerge.c b/drivers/xen/biomerge.c
index f3fbb700f5697..05a286d24f148 100644
--- a/drivers/xen/biomerge.c
+++ b/drivers/xen/biomerge.c
@@ -4,12 +4,13 @@
 #include 
 #include 
 
+/* check if @page can be merged with 'vec1' */
 bool xen_biovec_phys_mergeable(const struct bio_vec *vec1,
-  const struct bio_vec *vec2)
+  const struct page *page)
 {
 #if XEN_PAGE_SIZE == PAGE_SIZE
unsigned long bfn1 = pfn_to_bfn(page_to_pfn(vec1->bv_page));
-   unsigned long bfn2 = pfn_to_bfn(page_to_pfn(vec2->bv_page));
+   unsigned long bfn2 = pfn_to_bfn(page_to_pfn(page));
 
return bfn1 + PFN_DOWN(vec1->bv_offset + vec1->bv_len) == bfn2;
 #else
diff --git a/include/xen/xen.h b/include/xen/xen.h
index 0e2156786ad29..e1ba6921bc8eb 100644
--- a/include/xen/xen.h
+++ b/include/xen/xen.h
@@ -43,7 +43,9 @@ extern struct hvm_start_info pvh_start_info;
 #endif /* CONFIG_XEN_DOM0 */
 
 struct bio_vec;
+struct page;
+
 bool xen_biovec_phys_mergeable(const struct bio_vec *vec1,
-   const struct bio_vec *vec2);
+   const struct page *page);
 
 #endif /* _XEN_XEN_H */
-- 
2.20.1




___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH 5.1 280/405] block: pass page to xen_biovec_phys_mergeable

[Greg Kroah-Hartman]

[ Upstream commit 0383ad4374f7ad7edd925a2ee4753035c3f5508a ]

xen_biovec_phys_mergeable() only needs .bv_page of the 2nd bio bvec
for checking if the two bvecs can be merged, so pass page to
xen_biovec_phys_mergeable() directly.

No function change.

Cc: ris Ostrovsky 
Cc: Juergen Gross 
Cc: xen-devel@lists.xenproject.org
Cc: Omar Sandoval 
Cc: Christoph Hellwig 
Reviewed-by: Christoph Hellwig 
Reviewed-by: Boris Ostrovsky 
Signed-off-by: Ming Lei 
Signed-off-by: Jens Axboe 
Signed-off-by: Sasha Levin 
---
 block/blk.h| 2 +-
 drivers/xen/biomerge.c | 5 +++--
 include/xen/xen.h  | 4 +++-
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/block/blk.h b/block/blk.h
index 5d636ee416630..e27fd1512e4bb 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -75,7 +75,7 @@ static inline bool biovec_phys_mergeable(struct request_queue 
*q,
 
if (addr1 + vec1->bv_len != addr2)
return false;
-   if (xen_domain() && !xen_biovec_phys_mergeable(vec1, vec2))
+   if (xen_domain() && !xen_biovec_phys_mergeable(vec1, vec2->bv_page))
return false;
if ((addr1 | mask) != ((addr2 + vec2->bv_len - 1) | mask))
return false;
diff --git a/drivers/xen/biomerge.c b/drivers/xen/biomerge.c
index f3fbb700f5697..05a286d24f148 100644
--- a/drivers/xen/biomerge.c
+++ b/drivers/xen/biomerge.c
@@ -4,12 +4,13 @@
 #include 
 #include 
 
+/* check if @page can be merged with 'vec1' */
 bool xen_biovec_phys_mergeable(const struct bio_vec *vec1,
-  const struct bio_vec *vec2)
+  const struct page *page)
 {
 #if XEN_PAGE_SIZE == PAGE_SIZE
unsigned long bfn1 = pfn_to_bfn(page_to_pfn(vec1->bv_page));
-   unsigned long bfn2 = pfn_to_bfn(page_to_pfn(vec2->bv_page));
+   unsigned long bfn2 = pfn_to_bfn(page_to_pfn(page));
 
return bfn1 + PFN_DOWN(vec1->bv_offset + vec1->bv_len) == bfn2;
 #else
diff --git a/include/xen/xen.h b/include/xen/xen.h
index 19d032373de5a..19a72f591e2bd 100644
--- a/include/xen/xen.h
+++ b/include/xen/xen.h
@@ -43,8 +43,10 @@ extern struct hvm_start_info pvh_start_info;
 #endif /* CONFIG_XEN_DOM0 */
 
 struct bio_vec;
+struct page;
+
 bool xen_biovec_phys_mergeable(const struct bio_vec *vec1,
-   const struct bio_vec *vec2);
+   const struct page *page);
 
 #if defined(CONFIG_MEMORY_HOTPLUG) && defined(CONFIG_XEN_BALLOON)
 extern u64 xen_saved_max_mem_size;
-- 
2.20.1




___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH 5.1 279/405] block: avoid to break XEN by multi-page bvec

[Greg Kroah-Hartman]

[ Upstream commit db5ebd6edd2627d7e81a031643cf43587f63e66c ]

XEN has special page merge requirement, see xen_biovec_phys_mergeable().
We can't merge pages into one bvec simply for XEN.

So move XEN's specific check on page merge into __bio_try_merge_page(),
then abvoid to break XEN by multi-page bvec.

Cc: ris Ostrovsky 
Cc: xen-devel@lists.xenproject.org
Cc: Omar Sandoval 
Cc: Christoph Hellwig 
Reviewed-by: Juergen Gross 
Signed-off-by: Ming Lei 
Signed-off-by: Jens Axboe 
Signed-off-by: Sasha Levin 
---
 block/bio.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/block/bio.c b/block/bio.c
index 716510ecd7ffa..a3c80a6c1fe51 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -776,6 +776,8 @@ bool __bio_try_merge_page(struct bio *bio, struct page 
*page,
 
if (vec_end_addr + 1 != page_addr + off)
return false;
+   if (xen_domain() && !xen_biovec_phys_mergeable(bv, page))
+   return false;
if (same_page && (vec_end_addr & PAGE_MASK) != page_addr)
return false;
 
-- 
2.20.1




___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH 4.20 096/183] pvcalls-front: fix potential null dereference

[Greg Kroah-Hartman]

4.20-stable review patch.  If anyone has any objections, please let me know.

--

[ Upstream commit b4711098066f1cf808d4dc11a1a842860a3292fe ]

 static checker warning:
drivers/xen/pvcalls-front.c:373 alloc_active_ring()
error: we previously assumed 'map->active.ring' could be null
   (see line 357)

drivers/xen/pvcalls-front.c
351 static int alloc_active_ring(struct sock_mapping *map)
352 {
353 void *bytes;
354
355 map->active.ring = (struct pvcalls_data_intf *)
356 get_zeroed_page(GFP_KERNEL);
357 if (!map->active.ring)
^
Check

358 goto out;
359
360 map->active.ring->ring_order = PVCALLS_RING_ORDER;
361 bytes = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO,
362 PVCALLS_RING_ORDER);
363 if (!bytes)
364 goto out;
365
366 map->active.data.in = bytes;
367 map->active.data.out = bytes +
368 XEN_FLEX_RING_SIZE(PVCALLS_RING_ORDER);
369
370 return 0;
371
372 out:
--> 373 free_active_ring(map);
 ^^^
Add null check on map->active.ring before dereferencing it to avoid
any NULL pointer dereferences.

Fixes: 9f51c05dc41a ("pvcalls-front: Avoid get_free_pages(GFP_KERNEL) under 
spinlock")
Reported-by: Dan Carpenter 
Suggested-by: Boris Ostrovsky 
Signed-off-by: Wen Yang 
Reviewed-by: Boris Ostrovsky 
CC: Boris Ostrovsky 
CC: Juergen Gross 
CC: Stefano Stabellini 
CC: Dan Carpenter 
CC: xen-devel@lists.xenproject.org
CC: linux-ker...@vger.kernel.org
Signed-off-by: Boris Ostrovsky 
Signed-off-by: Sasha Levin 
---
 drivers/xen/pvcalls-front.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/xen/pvcalls-front.c b/drivers/xen/pvcalls-front.c
index 6357160d466ab..91da7e44d5d4f 100644
--- a/drivers/xen/pvcalls-front.c
+++ b/drivers/xen/pvcalls-front.c
@@ -343,6 +343,9 @@ int pvcalls_front_socket(struct socket *sock)
 
 static void free_active_ring(struct sock_mapping *map)
 {
+   if (!map->active.ring)
+   return;
+
free_pages((unsigned long)map->active.data.in,
map->active.ring->ring_order);
free_page((unsigned long)map->active.ring);
-- 
2.19.1




___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH 4.20 063/183] pvcalls-front: Avoid get_free_pages(GFP_KERNEL) under spinlock

[Greg Kroah-Hartman]

4.20-stable review patch.  If anyone has any objections, please let me know.

--

[ Upstream commit 9f51c05dc41a6d69423e3d03d18eb7ab22f9ec19 ]

The problem is that we call this with a spin lock held.
The call tree is:
pvcalls_front_accept() holds bedata->socket_lock.
-> create_active()
-> __get_free_pages() uses GFP_KERNEL

The create_active() function is only called from pvcalls_front_accept()
with a spin_lock held, The allocation is not allowed to sleep and
GFP_KERNEL is not sufficient.

This issue was detected by using the Coccinelle software.

v2: Add a function doing the allocations which is called
outside the lock and passing the allocated data to
create_active().

v3: Use the matching deallocators i.e., free_page()
and free_pages(), respectively.

v4: It would be better to pre-populate map (struct sock_mapping),
rather than introducing one more new struct.

v5: Since allocating the data outside of this call it should also
be freed outside, when create_active() fails.
Move kzalloc(sizeof(*map2), GFP_ATOMIC) outside spinlock and
use GFP_KERNEL instead.

v6: Drop the superfluous calls.

Suggested-by: Juergen Gross 
Suggested-by: Boris Ostrovsky 
Suggested-by: Stefano Stabellini 
Signed-off-by: Wen Yang 
Acked-by: Stefano Stabellini 
CC: Julia Lawall 
CC: Boris Ostrovsky 
CC: Juergen Gross 
CC: Stefano Stabellini 
CC: xen-devel@lists.xenproject.org
CC: linux-ker...@vger.kernel.org
Signed-off-by: Boris Ostrovsky 
Signed-off-by: Sasha Levin 
---
 drivers/xen/pvcalls-front.c | 81 +++--
 1 file changed, 59 insertions(+), 22 deletions(-)

diff --git a/drivers/xen/pvcalls-front.c b/drivers/xen/pvcalls-front.c
index 01588582ae663..6357160d466ab 100644
--- a/drivers/xen/pvcalls-front.c
+++ b/drivers/xen/pvcalls-front.c
@@ -341,6 +341,39 @@ int pvcalls_front_socket(struct socket *sock)
return ret;
 }
 
+static void free_active_ring(struct sock_mapping *map)
+{
+   free_pages((unsigned long)map->active.data.in,
+   map->active.ring->ring_order);
+   free_page((unsigned long)map->active.ring);
+}
+
+static int alloc_active_ring(struct sock_mapping *map)
+{
+   void *bytes;
+
+   map->active.ring = (struct pvcalls_data_intf *)
+   get_zeroed_page(GFP_KERNEL);
+   if (!map->active.ring)
+   goto out;
+
+   map->active.ring->ring_order = PVCALLS_RING_ORDER;
+   bytes = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO,
+   PVCALLS_RING_ORDER);
+   if (!bytes)
+   goto out;
+
+   map->active.data.in = bytes;
+   map->active.data.out = bytes +
+   XEN_FLEX_RING_SIZE(PVCALLS_RING_ORDER);
+
+   return 0;
+
+out:
+   free_active_ring(map);
+   return -ENOMEM;
+}
+
 static int create_active(struct sock_mapping *map, int *evtchn)
 {
void *bytes;
@@ -349,15 +382,7 @@ static int create_active(struct sock_mapping *map, int 
*evtchn)
*evtchn = -1;
init_waitqueue_head(&map->active.inflight_conn_req);
 
-   map->active.ring = (struct pvcalls_data_intf *)
-   __get_free_page(GFP_KERNEL | __GFP_ZERO);
-   if (map->active.ring == NULL)
-   goto out_error;
-   map->active.ring->ring_order = PVCALLS_RING_ORDER;
-   bytes = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO,
-   PVCALLS_RING_ORDER);
-   if (bytes == NULL)
-   goto out_error;
+   bytes = map->active.data.in;
for (i = 0; i < (1 << PVCALLS_RING_ORDER); i++)
map->active.ring->ref[i] = gnttab_grant_foreign_access(
pvcalls_front_dev->otherend_id,
@@ -367,10 +392,6 @@ static int create_active(struct sock_mapping *map, int 
*evtchn)
pvcalls_front_dev->otherend_id,
pfn_to_gfn(virt_to_pfn((void *)map->active.ring)), 0);
 
-   map->active.data.in = bytes;
-   map->active.data.out = bytes +
-   XEN_FLEX_RING_SIZE(PVCALLS_RING_ORDER);
-
ret = xenbus_alloc_evtchn(pvcalls_front_dev, evtchn);
if (ret)
goto out_error;
@@ -391,8 +412,6 @@ static int create_active(struct sock_mapping *map, int 
*evtchn)
 out_error:
if (*evtchn >= 0)
xenbus_free_evtchn(pvcalls_front_dev, *evtchn);
-   free_pages((unsigned long)map->active.data.in, PVCALLS_RING_ORDER);
-   free_page((unsigned long)map->active.ring);
return ret;
 }
 
@@ -412,17 +431,24 @@ int pvcalls_front_connect(struct socket *sock, struct 
sockaddr *addr,
return PTR_ERR(map);
 
bedata = dev_get_drvdata(&pvcalls_front_dev->dev);
+   ret = alloc_active_ring(map);
+   if (ret < 0) {
+   pvcalls_exit_sock(sock);
+   return ret;
+   }
 
spin_lock(&bedata->socket_lock);
ret = get_request(bedata, &req_id);
if (ret < 0) {
  

[Xen-devel] [PATCH 4.19 078/152] pvcalls-front: fix potential null dereference

[Greg Kroah-Hartman]

4.19-stable review patch.  If anyone has any objections, please let me know.

--

[ Upstream commit b4711098066f1cf808d4dc11a1a842860a3292fe ]

 static checker warning:
drivers/xen/pvcalls-front.c:373 alloc_active_ring()
error: we previously assumed 'map->active.ring' could be null
   (see line 357)

drivers/xen/pvcalls-front.c
351 static int alloc_active_ring(struct sock_mapping *map)
352 {
353 void *bytes;
354
355 map->active.ring = (struct pvcalls_data_intf *)
356 get_zeroed_page(GFP_KERNEL);
357 if (!map->active.ring)
^
Check

358 goto out;
359
360 map->active.ring->ring_order = PVCALLS_RING_ORDER;
361 bytes = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO,
362 PVCALLS_RING_ORDER);
363 if (!bytes)
364 goto out;
365
366 map->active.data.in = bytes;
367 map->active.data.out = bytes +
368 XEN_FLEX_RING_SIZE(PVCALLS_RING_ORDER);
369
370 return 0;
371
372 out:
--> 373 free_active_ring(map);
 ^^^
Add null check on map->active.ring before dereferencing it to avoid
any NULL pointer dereferences.

Fixes: 9f51c05dc41a ("pvcalls-front: Avoid get_free_pages(GFP_KERNEL) under 
spinlock")
Reported-by: Dan Carpenter 
Suggested-by: Boris Ostrovsky 
Signed-off-by: Wen Yang 
Reviewed-by: Boris Ostrovsky 
CC: Boris Ostrovsky 
CC: Juergen Gross 
CC: Stefano Stabellini 
CC: Dan Carpenter 
CC: xen-devel@lists.xenproject.org
CC: linux-ker...@vger.kernel.org
Signed-off-by: Boris Ostrovsky 
Signed-off-by: Sasha Levin 
---
 drivers/xen/pvcalls-front.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/xen/pvcalls-front.c b/drivers/xen/pvcalls-front.c
index 6357160d466ab..91da7e44d5d4f 100644
--- a/drivers/xen/pvcalls-front.c
+++ b/drivers/xen/pvcalls-front.c
@@ -343,6 +343,9 @@ int pvcalls_front_socket(struct socket *sock)
 
 static void free_active_ring(struct sock_mapping *map)
 {
+   if (!map->active.ring)
+   return;
+
free_pages((unsigned long)map->active.data.in,
map->active.ring->ring_order);
free_page((unsigned long)map->active.ring);
-- 
2.19.1




___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel