* [PATCH 0/2] Misc fixes for Xen grant DMA-mapping layer
@ 2022-10-05 17:48 Oleksandr Tyshchenko
2022-10-05 17:48 ` [PATCH 1/2] xen/virtio: Fix n_pages calculation in xen_grant_dma_map(unmap)_page() Oleksandr Tyshchenko
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Oleksandr Tyshchenko @ 2022-10-05 17:48 UTC (permalink / raw)
To: xen-devel, linux-kernel
Cc: Oleksandr Tyshchenko, Stefano Stabellini, Juergen Gross
From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
Hello all.
These are several fixes I collected when playing with virtio-net device
using Xen grant mappings.
Tested with both virtio-blk and virtio-net devices.
Oleksandr Tyshchenko (2):
xen/virtio: Fix n_pages calculation in xen_grant_dma_map(unmap)_page()
xen/virtio: Fix potential deadlock when accessing
xen_grant_dma_devices
drivers/xen/grant-dma-ops.c | 29 ++++++++++++++++++++++-------
1 file changed, 22 insertions(+), 7 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 1/2] xen/virtio: Fix n_pages calculation in xen_grant_dma_map(unmap)_page() 2022-10-05 17:48 [PATCH 0/2] Misc fixes for Xen grant DMA-mapping layer Oleksandr Tyshchenko @ 2022-10-05 17:48 ` Oleksandr Tyshchenko 2022-10-06 0:07 ` Stefano Stabellini ` (2 more replies) 2022-10-05 17:48 ` [PATCH 2/2] xen/virtio: Fix potential deadlock when accessing xen_grant_dma_devices Oleksandr Tyshchenko 2022-10-07 5:19 ` [PATCH 0/2] Misc fixes for Xen grant DMA-mapping layer Juergen Gross 2 siblings, 3 replies; 13+ messages in thread From: Oleksandr Tyshchenko @ 2022-10-05 17:48 UTC (permalink / raw) To: xen-devel, linux-kernel Cc: Oleksandr Tyshchenko, Stefano Stabellini, Juergen Gross From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> Take page offset into the account when calculating the number of pages to be granted. Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> Fixes: d6aca3504c7d ("xen/grant-dma-ops: Add option to restrict memory access under Xen") --- drivers/xen/grant-dma-ops.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/xen/grant-dma-ops.c b/drivers/xen/grant-dma-ops.c index 8973fc1e9ccc..1998d0e8ce82 100644 --- a/drivers/xen/grant-dma-ops.c +++ b/drivers/xen/grant-dma-ops.c @@ -153,7 +153,7 @@ static dma_addr_t xen_grant_dma_map_page(struct device *dev, struct page *page, unsigned long attrs) { struct xen_grant_dma_data *data; - unsigned int i, n_pages = PFN_UP(size); + unsigned int i, n_pages = PFN_UP(offset + size); grant_ref_t grant; dma_addr_t dma_handle; @@ -185,7 +185,8 @@ static void xen_grant_dma_unmap_page(struct device *dev, dma_addr_t dma_handle, unsigned long attrs) { struct xen_grant_dma_data *data; - unsigned int i, n_pages = PFN_UP(size); + unsigned long offset = dma_handle & (PAGE_SIZE - 1); + unsigned int i, n_pages = PFN_UP(offset + size); grant_ref_t grant; if (WARN_ON(dir == DMA_NONE)) -- 2.25.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] xen/virtio: Fix n_pages calculation in xen_grant_dma_map(unmap)_page() 2022-10-05 17:48 ` [PATCH 1/2] xen/virtio: Fix n_pages calculation in xen_grant_dma_map(unmap)_page() Oleksandr Tyshchenko @ 2022-10-06 0:07 ` Stefano Stabellini 2022-10-06 5:06 ` Juergen Gross 2022-10-06 7:35 ` Xenia Ragiadakou 2 siblings, 0 replies; 13+ messages in thread From: Stefano Stabellini @ 2022-10-06 0:07 UTC (permalink / raw) To: Oleksandr Tyshchenko Cc: xen-devel, linux-kernel, Oleksandr Tyshchenko, Stefano Stabellini, Juergen Gross On Wed, 5 Oct 2022, Oleksandr Tyshchenko wrote: > From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> > > Take page offset into the account when calculating the number of pages > to be granted. > > Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> > Fixes: d6aca3504c7d ("xen/grant-dma-ops: Add option to restrict memory access under Xen") Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > --- > drivers/xen/grant-dma-ops.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/xen/grant-dma-ops.c b/drivers/xen/grant-dma-ops.c > index 8973fc1e9ccc..1998d0e8ce82 100644 > --- a/drivers/xen/grant-dma-ops.c > +++ b/drivers/xen/grant-dma-ops.c > @@ -153,7 +153,7 @@ static dma_addr_t xen_grant_dma_map_page(struct device *dev, struct page *page, > unsigned long attrs) > { > struct xen_grant_dma_data *data; > - unsigned int i, n_pages = PFN_UP(size); > + unsigned int i, n_pages = PFN_UP(offset + size); > grant_ref_t grant; > dma_addr_t dma_handle; > > @@ -185,7 +185,8 @@ static void xen_grant_dma_unmap_page(struct device *dev, dma_addr_t dma_handle, > unsigned long attrs) > { > struct xen_grant_dma_data *data; > - unsigned int i, n_pages = PFN_UP(size); > + unsigned long offset = dma_handle & (PAGE_SIZE - 1); > + unsigned int i, n_pages = PFN_UP(offset + size); > grant_ref_t grant; > > if (WARN_ON(dir == DMA_NONE)) > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] xen/virtio: Fix n_pages calculation in xen_grant_dma_map(unmap)_page() 2022-10-05 17:48 ` [PATCH 1/2] xen/virtio: Fix n_pages calculation in xen_grant_dma_map(unmap)_page() Oleksandr Tyshchenko 2022-10-06 0:07 ` Stefano Stabellini @ 2022-10-06 5:06 ` Juergen Gross 2022-10-06 7:35 ` Xenia Ragiadakou 2 siblings, 0 replies; 13+ messages in thread From: Juergen Gross @ 2022-10-06 5:06 UTC (permalink / raw) To: Oleksandr Tyshchenko, xen-devel, linux-kernel Cc: Oleksandr Tyshchenko, Stefano Stabellini [-- Attachment #1.1.1: Type: text/plain, Size: 437 bytes --] On 05.10.22 19:48, Oleksandr Tyshchenko wrote: > From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> > > Take page offset into the account when calculating the number of pages > to be granted. > > Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> > Fixes: d6aca3504c7d ("xen/grant-dma-ops: Add option to restrict memory access under Xen") Reviewed-by: Juergen Gross <jgross@suse.com> Juergen [-- Attachment #1.1.2: OpenPGP public key --] [-- Type: application/pgp-keys, Size: 3149 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 495 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] xen/virtio: Fix n_pages calculation in xen_grant_dma_map(unmap)_page() 2022-10-05 17:48 ` [PATCH 1/2] xen/virtio: Fix n_pages calculation in xen_grant_dma_map(unmap)_page() Oleksandr Tyshchenko 2022-10-06 0:07 ` Stefano Stabellini 2022-10-06 5:06 ` Juergen Gross @ 2022-10-06 7:35 ` Xenia Ragiadakou 2022-10-06 8:05 ` Juergen Gross 2 siblings, 1 reply; 13+ messages in thread From: Xenia Ragiadakou @ 2022-10-06 7:35 UTC (permalink / raw) To: Oleksandr Tyshchenko, xen-devel, linux-kernel Cc: Oleksandr Tyshchenko, Stefano Stabellini, Juergen Gross On 10/5/22 20:48, Oleksandr Tyshchenko wrote: > From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> > > Take page offset into the account when calculating the number of pages > to be granted. > > Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> > Fixes: d6aca3504c7d ("xen/grant-dma-ops: Add option to restrict memory access under Xen") > --- > drivers/xen/grant-dma-ops.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/xen/grant-dma-ops.c b/drivers/xen/grant-dma-ops.c > index 8973fc1e9ccc..1998d0e8ce82 100644 > --- a/drivers/xen/grant-dma-ops.c > +++ b/drivers/xen/grant-dma-ops.c > @@ -153,7 +153,7 @@ static dma_addr_t xen_grant_dma_map_page(struct device *dev, struct page *page, > unsigned long attrs) > { > struct xen_grant_dma_data *data; > - unsigned int i, n_pages = PFN_UP(size); > + unsigned int i, n_pages = PFN_UP(offset + size); Here, why do we use PFN_UP and not XEN_PFN_UP? Also, since the variable 'n_pages' seems to refer to the number of grants (unless I got it all entirely wrong ...), wouldn't it be more suitable to call explicitly gnttab_count_grant()? If the above questions have been already answered in the past, please ignore. > grant_ref_t grant; > dma_addr_t dma_handle; > > @@ -185,7 +185,8 @@ static void xen_grant_dma_unmap_page(struct device *dev, dma_addr_t dma_handle, > unsigned long attrs) > { > struct xen_grant_dma_data *data; > - unsigned int i, n_pages = PFN_UP(size); > + unsigned long offset = dma_handle & (PAGE_SIZE - 1); > + unsigned int i, n_pages = PFN_UP(offset + size); > grant_ref_t grant; > > if (WARN_ON(dir == DMA_NONE)) -- Xenia ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] xen/virtio: Fix n_pages calculation in xen_grant_dma_map(unmap)_page() 2022-10-06 7:35 ` Xenia Ragiadakou @ 2022-10-06 8:05 ` Juergen Gross 2022-10-06 10:14 ` Oleksandr Tyshchenko 0 siblings, 1 reply; 13+ messages in thread From: Juergen Gross @ 2022-10-06 8:05 UTC (permalink / raw) To: Xenia Ragiadakou, Oleksandr Tyshchenko, xen-devel, linux-kernel Cc: Oleksandr Tyshchenko, Stefano Stabellini [-- Attachment #1.1.1: Type: text/plain, Size: 1482 bytes --] On 06.10.22 09:35, Xenia Ragiadakou wrote: > > On 10/5/22 20:48, Oleksandr Tyshchenko wrote: >> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> >> >> Take page offset into the account when calculating the number of pages >> to be granted. >> >> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> >> Fixes: d6aca3504c7d ("xen/grant-dma-ops: Add option to restrict memory access >> under Xen") >> --- >> drivers/xen/grant-dma-ops.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/xen/grant-dma-ops.c b/drivers/xen/grant-dma-ops.c >> index 8973fc1e9ccc..1998d0e8ce82 100644 >> --- a/drivers/xen/grant-dma-ops.c >> +++ b/drivers/xen/grant-dma-ops.c >> @@ -153,7 +153,7 @@ static dma_addr_t xen_grant_dma_map_page(struct device >> *dev, struct page *page, >> unsigned long attrs) >> { >> struct xen_grant_dma_data *data; >> - unsigned int i, n_pages = PFN_UP(size); >> + unsigned int i, n_pages = PFN_UP(offset + size); > > Here, why do we use PFN_UP and not XEN_PFN_UP? > Also, since the variable 'n_pages' seems to refer to the number of grants > (unless I got it all entirely wrong ...), wouldn't it be more suitable to call > explicitly gnttab_count_grant()? Good point. I think this will need another patch for switching grant-dma-ops.c to use XEN_PAGE_SIZE and XEN_PAGE_SHIFT. Juergen [-- Attachment #1.1.2: OpenPGP public key --] [-- Type: application/pgp-keys, Size: 3149 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 495 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] xen/virtio: Fix n_pages calculation in xen_grant_dma_map(unmap)_page() 2022-10-06 8:05 ` Juergen Gross @ 2022-10-06 10:14 ` Oleksandr Tyshchenko 2022-10-06 10:24 ` Juergen Gross 0 siblings, 1 reply; 13+ messages in thread From: Oleksandr Tyshchenko @ 2022-10-06 10:14 UTC (permalink / raw) To: Juergen Gross, Xenia Ragiadakou Cc: xen-devel, linux-kernel, Oleksandr Tyshchenko, Stefano Stabellini [-- Attachment #1: Type: text/plain, Size: 2056 bytes --] On Thu, Oct 6, 2022 at 11:05 AM Juergen Gross <jgross@suse.com> wrote: > On 06.10.22 09:35, Xenia Ragiadakou wrote: > Hello Xenia, Juergen [sorry for the possible format issues] > > > > On 10/5/22 20:48, Oleksandr Tyshchenko wrote: > >> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> > >> > >> Take page offset into the account when calculating the number of pages > >> to be granted. > >> > >> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> > >> Fixes: d6aca3504c7d ("xen/grant-dma-ops: Add option to restrict memory > access > >> under Xen") > >> --- > >> drivers/xen/grant-dma-ops.c | 5 +++-- > >> 1 file changed, 3 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/xen/grant-dma-ops.c b/drivers/xen/grant-dma-ops.c > >> index 8973fc1e9ccc..1998d0e8ce82 100644 > >> --- a/drivers/xen/grant-dma-ops.c > >> +++ b/drivers/xen/grant-dma-ops.c > >> @@ -153,7 +153,7 @@ static dma_addr_t xen_grant_dma_map_page(struct > device > >> *dev, struct page *page, > >> unsigned long attrs) > >> { > >> struct xen_grant_dma_data *data; > >> - unsigned int i, n_pages = PFN_UP(size); > >> + unsigned int i, n_pages = PFN_UP(offset + size); > > > > Here, why do we use PFN_UP and not XEN_PFN_UP? > > Also, since the variable 'n_pages' seems to refer to the number of > grants > > (unless I got it all entirely wrong ...), wouldn't it be more suitable > to call > > explicitly gnttab_count_grant()? > > Good point. > +1 > > I think this will need another patch for switching grant-dma-ops.c to > use XEN_PAGE_SIZE and XEN_PAGE_SHIFT. > +1 I can create a separate patch for converting on top of this series, it would be nice to clarify one point. So I will convert PAGE_SIZE/PAGE_SHIFT to XEN_PAGE_SIZE/XEN_PAGE_SHIFT respectively (where appropriate). Should the PFN_UP be converted to XEN_PFN_UP *or* use gnttab_count_grant() explicitly? Personally I would prefer the former, but would also be ok with the latter. > > > Juergen > > -- Regards, Oleksandr Tyshchenko [-- Attachment #2: Type: text/html, Size: 3925 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] xen/virtio: Fix n_pages calculation in xen_grant_dma_map(unmap)_page() 2022-10-06 10:14 ` Oleksandr Tyshchenko @ 2022-10-06 10:24 ` Juergen Gross 2022-10-06 10:30 ` Oleksandr Tyshchenko 0 siblings, 1 reply; 13+ messages in thread From: Juergen Gross @ 2022-10-06 10:24 UTC (permalink / raw) To: Oleksandr Tyshchenko, Xenia Ragiadakou Cc: xen-devel, linux-kernel, Oleksandr Tyshchenko, Stefano Stabellini [-- Attachment #1.1.1: Type: text/plain, Size: 2665 bytes --] On 06.10.22 12:14, Oleksandr Tyshchenko wrote: > > > On Thu, Oct 6, 2022 at 11:05 AM Juergen Gross <jgross@suse.com > <mailto:jgross@suse.com>> wrote: > > On 06.10.22 09:35, Xenia Ragiadakou wrote: > > > > Hello Xenia, Juergen > > [sorry for the possible format issues] > > > > > On 10/5/22 20:48, Oleksandr Tyshchenko wrote: > >> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com > <mailto:oleksandr_tyshchenko@epam.com>> > >> > >> Take page offset into the account when calculating the number of pages > >> to be granted. > >> > >> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com > <mailto:oleksandr_tyshchenko@epam.com>> > >> Fixes: d6aca3504c7d ("xen/grant-dma-ops: Add option to restrict memory > access > >> under Xen") > >> --- > >> drivers/xen/grant-dma-ops.c | 5 +++-- > >> 1 file changed, 3 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/xen/grant-dma-ops.c b/drivers/xen/grant-dma-ops.c > >> index 8973fc1e9ccc..1998d0e8ce82 100644 > >> --- a/drivers/xen/grant-dma-ops.c > >> +++ b/drivers/xen/grant-dma-ops.c > >> @@ -153,7 +153,7 @@ static dma_addr_t xen_grant_dma_map_page(struct device > >> *dev, struct page *page, > >> unsigned long attrs) > >> { > >> struct xen_grant_dma_data *data; > >> - unsigned int i, n_pages = PFN_UP(size); > >> + unsigned int i, n_pages = PFN_UP(offset + size); > > > > Here, why do we use PFN_UP and not XEN_PFN_UP? > > Also, since the variable 'n_pages' seems to refer to the number of grants > > (unless I got it all entirely wrong ...), wouldn't it be more suitable to > call > > explicitly gnttab_count_grant()? > > Good point. > > > +1 > > > I think this will need another patch for switching grant-dma-ops.c to > use XEN_PAGE_SIZE and XEN_PAGE_SHIFT. > > > +1 > > I can create a separate patch for converting on top of this series, it would be > nice to clarify one point. > > So I will convert PAGE_SIZE/PAGE_SHIFT to XEN_PAGE_SIZE/XEN_PAGE_SHIFT > respectively (where appropriate). Yes, that would be the idea. > Should the PFN_UP be converted to XEN_PFN_UP *or* use > gnttab_count_grant() explicitly? Personally I would prefer the former, but would > also be ok with the latter. I agree XEN_PFN_UP would be better, especially as XEN_PAGE_SIZE/XEN_PAGE_SHIFT will be used in the same functions. Juergen [-- Attachment #1.1.2: OpenPGP public key --] [-- Type: application/pgp-keys, Size: 3149 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 495 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] xen/virtio: Fix n_pages calculation in xen_grant_dma_map(unmap)_page() 2022-10-06 10:24 ` Juergen Gross @ 2022-10-06 10:30 ` Oleksandr Tyshchenko 0 siblings, 0 replies; 13+ messages in thread From: Oleksandr Tyshchenko @ 2022-10-06 10:30 UTC (permalink / raw) To: Juergen Gross Cc: Xenia Ragiadakou, xen-devel, linux-kernel, Oleksandr Tyshchenko, Stefano Stabellini [-- Attachment #1: Type: text/plain, Size: 2896 bytes --] On Thu, Oct 6, 2022 at 1:24 PM Juergen Gross <jgross@suse.com> wrote: Hello Juergen [sorry for the possible format issues] On 06.10.22 12:14, Oleksandr Tyshchenko wrote: > > > > > > On Thu, Oct 6, 2022 at 11:05 AM Juergen Gross <jgross@suse.com > > <mailto:jgross@suse.com>> wrote: > > > > On 06.10.22 09:35, Xenia Ragiadakou wrote: > > > > > > > > Hello Xenia, Juergen > > > > [sorry for the possible format issues] > > > > > > > > On 10/5/22 20:48, Oleksandr Tyshchenko wrote: > > >> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com > > <mailto:oleksandr_tyshchenko@epam.com>> > > >> > > >> Take page offset into the account when calculating the number of > pages > > >> to be granted. > > >> > > >> Signed-off-by: Oleksandr Tyshchenko < > oleksandr_tyshchenko@epam.com > > <mailto:oleksandr_tyshchenko@epam.com>> > > >> Fixes: d6aca3504c7d ("xen/grant-dma-ops: Add option to restrict > memory > > access > > >> under Xen") > > >> --- > > >> drivers/xen/grant-dma-ops.c | 5 +++-- > > >> 1 file changed, 3 insertions(+), 2 deletions(-) > > >> > > >> diff --git a/drivers/xen/grant-dma-ops.c > b/drivers/xen/grant-dma-ops.c > > >> index 8973fc1e9ccc..1998d0e8ce82 100644 > > >> --- a/drivers/xen/grant-dma-ops.c > > >> +++ b/drivers/xen/grant-dma-ops.c > > >> @@ -153,7 +153,7 @@ static dma_addr_t > xen_grant_dma_map_page(struct device > > >> *dev, struct page *page, > > >> unsigned long attrs) > > >> { > > >> struct xen_grant_dma_data *data; > > >> - unsigned int i, n_pages = PFN_UP(size); > > >> + unsigned int i, n_pages = PFN_UP(offset + size); > > > > > > Here, why do we use PFN_UP and not XEN_PFN_UP? > > > Also, since the variable 'n_pages' seems to refer to the number > of grants > > > (unless I got it all entirely wrong ...), wouldn't it be more > suitable to > > call > > > explicitly gnttab_count_grant()? > > > > Good point. > > > > > > +1 > > > > > > I think this will need another patch for switching grant-dma-ops.c to > > use XEN_PAGE_SIZE and XEN_PAGE_SHIFT. > > > > > > +1 > > > > I can create a separate patch for converting on top of this series, it > would be > > nice to clarify one point. > > > > So I will convert PAGE_SIZE/PAGE_SHIFT to XEN_PAGE_SIZE/XEN_PAGE_SHIFT > > respectively (where appropriate). > > Yes, that would be the idea. > > > Should the PFN_UP be converted to XEN_PFN_UP *or* use > > gnttab_count_grant() explicitly? Personally I would prefer the former, > but would > > also be ok with the latter. > > I agree XEN_PFN_UP would be better, especially as > XEN_PAGE_SIZE/XEN_PAGE_SHIFT > will be used in the same functions. > Thanks for the clarification. > > > Juergen > -- Regards, Oleksandr Tyshchenko [-- Attachment #2: Type: text/html, Size: 5237 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/2] xen/virtio: Fix potential deadlock when accessing xen_grant_dma_devices 2022-10-05 17:48 [PATCH 0/2] Misc fixes for Xen grant DMA-mapping layer Oleksandr Tyshchenko 2022-10-05 17:48 ` [PATCH 1/2] xen/virtio: Fix n_pages calculation in xen_grant_dma_map(unmap)_page() Oleksandr Tyshchenko @ 2022-10-05 17:48 ` Oleksandr Tyshchenko 2022-10-06 0:19 ` Stefano Stabellini 2022-10-06 5:08 ` Juergen Gross 2022-10-07 5:19 ` [PATCH 0/2] Misc fixes for Xen grant DMA-mapping layer Juergen Gross 2 siblings, 2 replies; 13+ messages in thread From: Oleksandr Tyshchenko @ 2022-10-05 17:48 UTC (permalink / raw) To: xen-devel, linux-kernel Cc: Oleksandr Tyshchenko, Stefano Stabellini, Juergen Gross From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> As find_xen_grant_dma_data() is called from both interrupt and process contexts, the access to xen_grant_dma_devices XArray must be protected by xa_lock_irqsave to avoid deadlock scenario. As XArray API doesn't provide xa_store_irqsave helper, call lockless __xa_store directly and guard it externally. Also move the storage of the XArray's entry to a separate helper. Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> Fixes: d6aca3504c7d ("xen/grant-dma-ops: Add option to restrict memory access under Xen") --- drivers/xen/grant-dma-ops.c | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/drivers/xen/grant-dma-ops.c b/drivers/xen/grant-dma-ops.c index 1998d0e8ce82..c66f56d24013 100644 --- a/drivers/xen/grant-dma-ops.c +++ b/drivers/xen/grant-dma-ops.c @@ -25,7 +25,7 @@ struct xen_grant_dma_data { bool broken; }; -static DEFINE_XARRAY(xen_grant_dma_devices); +static DEFINE_XARRAY_FLAGS(xen_grant_dma_devices, XA_FLAGS_LOCK_IRQ); #define XEN_GRANT_DMA_ADDR_OFF (1ULL << 63) @@ -42,14 +42,29 @@ static inline grant_ref_t dma_to_grant(dma_addr_t dma) static struct xen_grant_dma_data *find_xen_grant_dma_data(struct device *dev) { struct xen_grant_dma_data *data; + unsigned long flags; - xa_lock(&xen_grant_dma_devices); + xa_lock_irqsave(&xen_grant_dma_devices, flags); data = xa_load(&xen_grant_dma_devices, (unsigned long)dev); - xa_unlock(&xen_grant_dma_devices); + xa_unlock_irqrestore(&xen_grant_dma_devices, flags); return data; } +static int store_xen_grant_dma_data(struct device *dev, + struct xen_grant_dma_data *data) +{ + unsigned long flags; + int ret; + + xa_lock_irqsave(&xen_grant_dma_devices, flags); + ret = xa_err(__xa_store(&xen_grant_dma_devices, (unsigned long)dev, data, + GFP_ATOMIC)); + xa_unlock_irqrestore(&xen_grant_dma_devices, flags); + + return ret; +} + /* * DMA ops for Xen frontends (e.g. virtio). * @@ -338,8 +353,7 @@ void xen_grant_setup_dma_ops(struct device *dev) */ data->backend_domid = iommu_spec.args[0]; - if (xa_err(xa_store(&xen_grant_dma_devices, (unsigned long)dev, data, - GFP_KERNEL))) { + if (store_xen_grant_dma_data(dev, data)) { dev_err(dev, "Cannot store Xen grant DMA data\n"); goto err; } -- 2.25.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] xen/virtio: Fix potential deadlock when accessing xen_grant_dma_devices 2022-10-05 17:48 ` [PATCH 2/2] xen/virtio: Fix potential deadlock when accessing xen_grant_dma_devices Oleksandr Tyshchenko @ 2022-10-06 0:19 ` Stefano Stabellini 2022-10-06 5:08 ` Juergen Gross 1 sibling, 0 replies; 13+ messages in thread From: Stefano Stabellini @ 2022-10-06 0:19 UTC (permalink / raw) To: Oleksandr Tyshchenko Cc: xen-devel, linux-kernel, Oleksandr Tyshchenko, Stefano Stabellini, Juergen Gross On Wed, 5 Oct 2022, Oleksandr Tyshchenko wrote: > From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> > > As find_xen_grant_dma_data() is called from both interrupt and process > contexts, the access to xen_grant_dma_devices XArray must be protected > by xa_lock_irqsave to avoid deadlock scenario. > As XArray API doesn't provide xa_store_irqsave helper, call lockless > __xa_store directly and guard it externally. > > Also move the storage of the XArray's entry to a separate helper. > > Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> > Fixes: d6aca3504c7d ("xen/grant-dma-ops: Add option to restrict memory access under Xen") Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > --- > drivers/xen/grant-dma-ops.c | 24 +++++++++++++++++++----- > 1 file changed, 19 insertions(+), 5 deletions(-) > > diff --git a/drivers/xen/grant-dma-ops.c b/drivers/xen/grant-dma-ops.c > index 1998d0e8ce82..c66f56d24013 100644 > --- a/drivers/xen/grant-dma-ops.c > +++ b/drivers/xen/grant-dma-ops.c > @@ -25,7 +25,7 @@ struct xen_grant_dma_data { > bool broken; > }; > > -static DEFINE_XARRAY(xen_grant_dma_devices); > +static DEFINE_XARRAY_FLAGS(xen_grant_dma_devices, XA_FLAGS_LOCK_IRQ); > > #define XEN_GRANT_DMA_ADDR_OFF (1ULL << 63) > > @@ -42,14 +42,29 @@ static inline grant_ref_t dma_to_grant(dma_addr_t dma) > static struct xen_grant_dma_data *find_xen_grant_dma_data(struct device *dev) > { > struct xen_grant_dma_data *data; > + unsigned long flags; > > - xa_lock(&xen_grant_dma_devices); > + xa_lock_irqsave(&xen_grant_dma_devices, flags); > data = xa_load(&xen_grant_dma_devices, (unsigned long)dev); > - xa_unlock(&xen_grant_dma_devices); > + xa_unlock_irqrestore(&xen_grant_dma_devices, flags); > > return data; > } > > +static int store_xen_grant_dma_data(struct device *dev, > + struct xen_grant_dma_data *data) > +{ > + unsigned long flags; > + int ret; > + > + xa_lock_irqsave(&xen_grant_dma_devices, flags); > + ret = xa_err(__xa_store(&xen_grant_dma_devices, (unsigned long)dev, data, > + GFP_ATOMIC)); > + xa_unlock_irqrestore(&xen_grant_dma_devices, flags); > + > + return ret; > +} > + > /* > * DMA ops for Xen frontends (e.g. virtio). > * > @@ -338,8 +353,7 @@ void xen_grant_setup_dma_ops(struct device *dev) > */ > data->backend_domid = iommu_spec.args[0]; > > - if (xa_err(xa_store(&xen_grant_dma_devices, (unsigned long)dev, data, > - GFP_KERNEL))) { > + if (store_xen_grant_dma_data(dev, data)) { > dev_err(dev, "Cannot store Xen grant DMA data\n"); > goto err; > } > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] xen/virtio: Fix potential deadlock when accessing xen_grant_dma_devices 2022-10-05 17:48 ` [PATCH 2/2] xen/virtio: Fix potential deadlock when accessing xen_grant_dma_devices Oleksandr Tyshchenko 2022-10-06 0:19 ` Stefano Stabellini @ 2022-10-06 5:08 ` Juergen Gross 1 sibling, 0 replies; 13+ messages in thread From: Juergen Gross @ 2022-10-06 5:08 UTC (permalink / raw) To: Oleksandr Tyshchenko, xen-devel, linux-kernel Cc: Oleksandr Tyshchenko, Stefano Stabellini [-- Attachment #1.1.1: Type: text/plain, Size: 734 bytes --] On 05.10.22 19:48, Oleksandr Tyshchenko wrote: > From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> > > As find_xen_grant_dma_data() is called from both interrupt and process > contexts, the access to xen_grant_dma_devices XArray must be protected > by xa_lock_irqsave to avoid deadlock scenario. > As XArray API doesn't provide xa_store_irqsave helper, call lockless > __xa_store directly and guard it externally. > > Also move the storage of the XArray's entry to a separate helper. > > Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> > Fixes: d6aca3504c7d ("xen/grant-dma-ops: Add option to restrict memory access under Xen") Reviewed-by: Juergen Gross <jgross@suse.com> Juergen [-- Attachment #1.1.2: OpenPGP public key --] [-- Type: application/pgp-keys, Size: 3149 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 495 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/2] Misc fixes for Xen grant DMA-mapping layer 2022-10-05 17:48 [PATCH 0/2] Misc fixes for Xen grant DMA-mapping layer Oleksandr Tyshchenko 2022-10-05 17:48 ` [PATCH 1/2] xen/virtio: Fix n_pages calculation in xen_grant_dma_map(unmap)_page() Oleksandr Tyshchenko 2022-10-05 17:48 ` [PATCH 2/2] xen/virtio: Fix potential deadlock when accessing xen_grant_dma_devices Oleksandr Tyshchenko @ 2022-10-07 5:19 ` Juergen Gross 2 siblings, 0 replies; 13+ messages in thread From: Juergen Gross @ 2022-10-07 5:19 UTC (permalink / raw) To: Oleksandr Tyshchenko, xen-devel, linux-kernel Cc: Oleksandr Tyshchenko, Stefano Stabellini [-- Attachment #1.1.1: Type: text/plain, Size: 680 bytes --] On 05.10.22 19:48, Oleksandr Tyshchenko wrote: > From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> > > Hello all. > > These are several fixes I collected when playing with virtio-net device > using Xen grant mappings. > > Tested with both virtio-blk and virtio-net devices. > > Oleksandr Tyshchenko (2): > xen/virtio: Fix n_pages calculation in xen_grant_dma_map(unmap)_page() > xen/virtio: Fix potential deadlock when accessing > xen_grant_dma_devices > > drivers/xen/grant-dma-ops.c | 29 ++++++++++++++++++++++------- > 1 file changed, 22 insertions(+), 7 deletions(-) > Series pushed to xen/tip.git for-linus-6.1 Juergen [-- Attachment #1.1.2: OpenPGP public key --] [-- Type: application/pgp-keys, Size: 3149 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 495 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2022-10-07 5:19 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-10-05 17:48 [PATCH 0/2] Misc fixes for Xen grant DMA-mapping layer Oleksandr Tyshchenko 2022-10-05 17:48 ` [PATCH 1/2] xen/virtio: Fix n_pages calculation in xen_grant_dma_map(unmap)_page() Oleksandr Tyshchenko 2022-10-06 0:07 ` Stefano Stabellini 2022-10-06 5:06 ` Juergen Gross 2022-10-06 7:35 ` Xenia Ragiadakou 2022-10-06 8:05 ` Juergen Gross 2022-10-06 10:14 ` Oleksandr Tyshchenko 2022-10-06 10:24 ` Juergen Gross 2022-10-06 10:30 ` Oleksandr Tyshchenko 2022-10-05 17:48 ` [PATCH 2/2] xen/virtio: Fix potential deadlock when accessing xen_grant_dma_devices Oleksandr Tyshchenko 2022-10-06 0:19 ` Stefano Stabellini 2022-10-06 5:08 ` Juergen Gross 2022-10-07 5:19 ` [PATCH 0/2] Misc fixes for Xen grant DMA-mapping layer Juergen Gross
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.