On Sat, Jan 6, 2018 at 6:00 PM, Hans de Goede hdegoede@redhat.com wrote:
Hi,
On 06-01-18 15:20, Alexander Kapshuk wrote:
On Mon, Dec 25, 2017 at 04:42:59PM +0200, Alexander Kapshuk wrote:
Sparse emitted the following warning: ../drivers/staging/vboxvideo/vbox_fb.c:173:27: warning: incorrect type in assignment (different address spaces) ../drivers/staging/vboxvideo/vbox_fb.c:173:27: expected char [noderef] asn:2*screen_base ../drivers/staging/vboxvideo/vbox_fb.c:173:27: got void *virtual
The vbox_bo buffer object kernel mapping is handled by a call to ttm_bo_kmap() prior to the assignment of bo->kmap.virtual to info->screen_base of type char __iomem*. Casting bo->kmap.virtual to char __iomem* in this assignment fixes the warning.
vboxvideo: Fix address space of expression removal sparse warning
Sparse emitted the following warning: ../drivers/staging/vboxvideo/vbox_main.c:64:25: warning: cast removes address space of expression
vbox->vbva_buffers iomapping is handled by calling vbox_accel_init() from vbox_hw_init(). __force attribute is used in assignment to vbva to fix the warning.
Signed-off-by: Alexander Kapshuk alexander.kapshuk@gmail.com
drivers/staging/vboxvideo/vbox_fb.c | 2 +- drivers/staging/vboxvideo/vbox_main.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/vboxvideo/vbox_fb.c b/drivers/staging/vboxvideo/vbox_fb.c index 8aed248db6e2..43c39eca4ae1 100644 --- a/drivers/staging/vboxvideo/vbox_fb.c +++ b/drivers/staging/vboxvideo/vbox_fb.c @@ -170,7 +170,7 @@ static int vboxfb_create(struct drm_fb_helper *helper, drm_fb_helper_fill_var(info, &fbdev->helper, sizes->fb_width, sizes->fb_height);
info->screen_base = bo->kmap.virtual;
info->screen_base = (char __iomem *)bo->kmap.virtual; info->screen_size = size;
#ifdef CONFIG_DRM_KMS_FB_HELPER
This fix looks good to me.
diff --git a/drivers/staging/vboxvideo/vbox_main.c b/drivers/staging/vboxvideo/vbox_main.c index 80bd039fa08e..973b3bcc04b1 100644 --- a/drivers/staging/vboxvideo/vbox_main.c +++ b/drivers/staging/vboxvideo/vbox_main.c @@ -61,7 +61,7 @@ void vbox_enable_accel(struct vbox_private *vbox) if (vbox->vbva_info[i].vbva) continue;
vbva = (void *)vbox->vbva_buffers + i *
VBVA_MIN_BUFFER_SIZE;
vbva = (void __force *)vbox->vbva_buffers + i *
VBVA_MIN_BUFFER_SIZE; if (!vbva_enable(&vbox->vbva_info[i], vbox->guest_pool, vbva, i)) { /* very old host or driver error. */
Hmm, isn't there a cleaner way to fix this ? Maybe make vbva_enable's argument (and the local vbva variable) of type u8 __iomem * too ?
Regards,
Hans
Hi Hans,
Thanks for your prompt response.
I had a good look at the vbva_enable() function's definition and to the best of my knowledge, changing vbva's type from 'struct vbva_buffer *' to 'u8 __iomem *' wouldn't work. vbva_enable() relies on vbva's type being a pointer to 'struct vbva_buffer': vbva's memory gets set to zero; some of vbva's members are initialised to particular values; vbva_ctx->vbva expects vbva to be a pointer to 'struct vbva_buffer' as well;
Or am I misreading this? What are your thoughts on this? Thanks.
Alexander.
HI,
On 06-01-18 20:30, Alexander Kapshuk wrote:
On Sat, Jan 6, 2018 at 6:00 PM, Hans de Goede hdegoede@redhat.com wrote:
Hi,
On 06-01-18 15:20, Alexander Kapshuk wrote:
On Mon, Dec 25, 2017 at 04:42:59PM +0200, Alexander Kapshuk wrote:
Sparse emitted the following warning: ../drivers/staging/vboxvideo/vbox_fb.c:173:27: warning: incorrect type in assignment (different address spaces) ../drivers/staging/vboxvideo/vbox_fb.c:173:27: expected char [noderef] asn:2*screen_base ../drivers/staging/vboxvideo/vbox_fb.c:173:27: got void *virtual
The vbox_bo buffer object kernel mapping is handled by a call to ttm_bo_kmap() prior to the assignment of bo->kmap.virtual to info->screen_base of type char __iomem*. Casting bo->kmap.virtual to char __iomem* in this assignment fixes the warning.
vboxvideo: Fix address space of expression removal sparse warning
Sparse emitted the following warning: ../drivers/staging/vboxvideo/vbox_main.c:64:25: warning: cast removes address space of expression
vbox->vbva_buffers iomapping is handled by calling vbox_accel_init() from vbox_hw_init(). __force attribute is used in assignment to vbva to fix the warning.
Signed-off-by: Alexander Kapshuk alexander.kapshuk@gmail.com
drivers/staging/vboxvideo/vbox_fb.c | 2 +- drivers/staging/vboxvideo/vbox_main.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/vboxvideo/vbox_fb.c b/drivers/staging/vboxvideo/vbox_fb.c index 8aed248db6e2..43c39eca4ae1 100644 --- a/drivers/staging/vboxvideo/vbox_fb.c +++ b/drivers/staging/vboxvideo/vbox_fb.c @@ -170,7 +170,7 @@ static int vboxfb_create(struct drm_fb_helper *helper, drm_fb_helper_fill_var(info, &fbdev->helper, sizes->fb_width, sizes->fb_height);
info->screen_base = bo->kmap.virtual;
info->screen_base = (char __iomem *)bo->kmap.virtual; info->screen_size = size;
#ifdef CONFIG_DRM_KMS_FB_HELPER
This fix looks good to me.
diff --git a/drivers/staging/vboxvideo/vbox_main.c b/drivers/staging/vboxvideo/vbox_main.c index 80bd039fa08e..973b3bcc04b1 100644 --- a/drivers/staging/vboxvideo/vbox_main.c +++ b/drivers/staging/vboxvideo/vbox_main.c @@ -61,7 +61,7 @@ void vbox_enable_accel(struct vbox_private *vbox) if (vbox->vbva_info[i].vbva) continue;
vbva = (void *)vbox->vbva_buffers + i *
VBVA_MIN_BUFFER_SIZE;
vbva = (void __force *)vbox->vbva_buffers + i *
VBVA_MIN_BUFFER_SIZE; if (!vbva_enable(&vbox->vbva_info[i], vbox->guest_pool, vbva, i)) { /* very old host or driver error. */
Hmm, isn't there a cleaner way to fix this ? Maybe make vbva_enable's argument (and the local vbva variable) of type u8 __iomem * too ?
Regards,
Hans
Hi Hans,
Thanks for your prompt response.
I had a good look at the vbva_enable() function's definition and to the best of my knowledge, changing vbva's type from 'struct vbva_buffer *' to 'u8 __iomem *' wouldn't work. vbva_enable() relies on vbva's type being a pointer to 'struct vbva_buffer': vbva's memory gets set to zero; some of vbva's members are initialised to particular values; vbva_ctx->vbva expects vbva to be a pointer to 'struct vbva_buffer' as well;
Or am I misreading this?
No your not misreading this I did not check the function myself before commenting myself, my bad.
What are your thoughts on this?
Lets just move ahead with your original fix:
Reviewed-by: Hans de Goede hdegoede@redhat.com
Regards,
Hans
On Sat, Jan 6, 2018 at 9:43 PM, Hans de Goede hdegoede@redhat.com wrote:
HI,
On 06-01-18 20:30, Alexander Kapshuk wrote:
On Sat, Jan 6, 2018 at 6:00 PM, Hans de Goede hdegoede@redhat.com wrote:
Hi,
On 06-01-18 15:20, Alexander Kapshuk wrote:
On Mon, Dec 25, 2017 at 04:42:59PM +0200, Alexander Kapshuk wrote:
Sparse emitted the following warning: ../drivers/staging/vboxvideo/vbox_fb.c:173:27: warning: incorrect type in assignment (different address spaces) ../drivers/staging/vboxvideo/vbox_fb.c:173:27: expected char [noderef] asn:2*screen_base ../drivers/staging/vboxvideo/vbox_fb.c:173:27: got void *virtual
The vbox_bo buffer object kernel mapping is handled by a call to ttm_bo_kmap() prior to the assignment of bo->kmap.virtual to info->screen_base of type char __iomem*. Casting bo->kmap.virtual to char __iomem* in this assignment fixes the warning.
vboxvideo: Fix address space of expression removal sparse warning
Sparse emitted the following warning: ../drivers/staging/vboxvideo/vbox_main.c:64:25: warning: cast removes address space of expression
vbox->vbva_buffers iomapping is handled by calling vbox_accel_init() from vbox_hw_init(). __force attribute is used in assignment to vbva to fix the warning.
Signed-off-by: Alexander Kapshuk alexander.kapshuk@gmail.com
drivers/staging/vboxvideo/vbox_fb.c | 2 +- drivers/staging/vboxvideo/vbox_main.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/vboxvideo/vbox_fb.c b/drivers/staging/vboxvideo/vbox_fb.c index 8aed248db6e2..43c39eca4ae1 100644 --- a/drivers/staging/vboxvideo/vbox_fb.c +++ b/drivers/staging/vboxvideo/vbox_fb.c @@ -170,7 +170,7 @@ static int vboxfb_create(struct drm_fb_helper *helper, drm_fb_helper_fill_var(info, &fbdev->helper, sizes->fb_width, sizes->fb_height);
info->screen_base = bo->kmap.virtual;
info->screen_base = (char __iomem *)bo->kmap.virtual; info->screen_size = size;
#ifdef CONFIG_DRM_KMS_FB_HELPER
This fix looks good to me.
diff --git a/drivers/staging/vboxvideo/vbox_main.c b/drivers/staging/vboxvideo/vbox_main.c index 80bd039fa08e..973b3bcc04b1 100644 --- a/drivers/staging/vboxvideo/vbox_main.c +++ b/drivers/staging/vboxvideo/vbox_main.c @@ -61,7 +61,7 @@ void vbox_enable_accel(struct vbox_private *vbox) if (vbox->vbva_info[i].vbva) continue;
vbva = (void *)vbox->vbva_buffers + i *
VBVA_MIN_BUFFER_SIZE;
vbva = (void __force *)vbox->vbva_buffers + i *
VBVA_MIN_BUFFER_SIZE; if (!vbva_enable(&vbox->vbva_info[i], vbox->guest_pool, vbva, i)) { /* very old host or driver error. */
Hmm, isn't there a cleaner way to fix this ? Maybe make vbva_enable's argument (and the local vbva variable) of type u8 __iomem * too ?
Regards,
Hans
Hi Hans,
Thanks for your prompt response.
I had a good look at the vbva_enable() function's definition and to the best of my knowledge, changing vbva's type from 'struct vbva_buffer *' to 'u8 __iomem *' wouldn't work. vbva_enable() relies on vbva's type being a pointer to 'struct vbva_buffer': vbva's memory gets set to zero; some of vbva's members are initialised to particular values; vbva_ctx->vbva expects vbva to be a pointer to 'struct vbva_buffer' as well;
Or am I misreading this?
No your not misreading this I did not check the function myself before commenting myself, my bad.
Thanks for confirming my understanding of this.
What are your thoughts on this?
Lets just move ahead with your original fix:
Did you want me to resend the patch, or is someone going to pull the original one I sent into their tree? Thanks.
Reviewed-by: Hans de Goede hdegoede@redhat.com
Regards,
Hans
Hi,
On 06-01-18 20:56, Alexander Kapshuk wrote:
On Sat, Jan 6, 2018 at 9:43 PM, Hans de Goede hdegoede@redhat.com wrote:
HI,
On 06-01-18 20:30, Alexander Kapshuk wrote:
On Sat, Jan 6, 2018 at 6:00 PM, Hans de Goede hdegoede@redhat.com wrote:
Hi,
On 06-01-18 15:20, Alexander Kapshuk wrote:
On Mon, Dec 25, 2017 at 04:42:59PM +0200, Alexander Kapshuk wrote:
Sparse emitted the following warning: ../drivers/staging/vboxvideo/vbox_fb.c:173:27: warning: incorrect type in assignment (different address spaces) ../drivers/staging/vboxvideo/vbox_fb.c:173:27: expected char [noderef] asn:2*screen_base ../drivers/staging/vboxvideo/vbox_fb.c:173:27: got void *virtual
The vbox_bo buffer object kernel mapping is handled by a call to ttm_bo_kmap() prior to the assignment of bo->kmap.virtual to info->screen_base of type char __iomem*. Casting bo->kmap.virtual to char __iomem* in this assignment fixes the warning.
vboxvideo: Fix address space of expression removal sparse warning
Sparse emitted the following warning: ../drivers/staging/vboxvideo/vbox_main.c:64:25: warning: cast removes address space of expression
vbox->vbva_buffers iomapping is handled by calling vbox_accel_init() from vbox_hw_init(). __force attribute is used in assignment to vbva to fix the warning.
Signed-off-by: Alexander Kapshuk alexander.kapshuk@gmail.com
drivers/staging/vboxvideo/vbox_fb.c | 2 +- drivers/staging/vboxvideo/vbox_main.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/vboxvideo/vbox_fb.c b/drivers/staging/vboxvideo/vbox_fb.c index 8aed248db6e2..43c39eca4ae1 100644 --- a/drivers/staging/vboxvideo/vbox_fb.c +++ b/drivers/staging/vboxvideo/vbox_fb.c @@ -170,7 +170,7 @@ static int vboxfb_create(struct drm_fb_helper *helper, drm_fb_helper_fill_var(info, &fbdev->helper, sizes->fb_width, sizes->fb_height);
info->screen_base = bo->kmap.virtual;
info->screen_base = (char __iomem *)bo->kmap.virtual; info->screen_size = size;
#ifdef CONFIG_DRM_KMS_FB_HELPER
This fix looks good to me.
diff --git a/drivers/staging/vboxvideo/vbox_main.c b/drivers/staging/vboxvideo/vbox_main.c index 80bd039fa08e..973b3bcc04b1 100644 --- a/drivers/staging/vboxvideo/vbox_main.c +++ b/drivers/staging/vboxvideo/vbox_main.c @@ -61,7 +61,7 @@ void vbox_enable_accel(struct vbox_private *vbox) if (vbox->vbva_info[i].vbva) continue;
vbva = (void *)vbox->vbva_buffers + i *
VBVA_MIN_BUFFER_SIZE;
vbva = (void __force *)vbox->vbva_buffers + i *
VBVA_MIN_BUFFER_SIZE; if (!vbva_enable(&vbox->vbva_info[i], vbox->guest_pool, vbva, i)) { /* very old host or driver error. */
Hmm, isn't there a cleaner way to fix this ? Maybe make vbva_enable's argument (and the local vbva variable) of type u8 __iomem * too ?
Regards,
Hans
Hi Hans,
Thanks for your prompt response.
I had a good look at the vbva_enable() function's definition and to the best of my knowledge, changing vbva's type from 'struct vbva_buffer *' to 'u8 __iomem *' wouldn't work. vbva_enable() relies on vbva's type being a pointer to 'struct vbva_buffer': vbva's memory gets set to zero; some of vbva's members are initialised to particular values; vbva_ctx->vbva expects vbva to be a pointer to 'struct vbva_buffer' as well;
Or am I misreading this?
No your not misreading this I did not check the function myself before commenting myself, my bad.
Thanks for confirming my understanding of this.
What are your thoughts on this?
Lets just move ahead with your original fix:
Did you want me to resend the patch, or is someone going to pull the original one I sent into their tree? Thanks.
I expect Greg to pick it up without a resend. But with all the Spectre stuff going on right now he might miss it, so I would say give it 14 days and if he has not picked it up by then, resend it with my reviewed-by added.
Regards,
Hans
On Sat, Jan 6, 2018 at 10:00 PM, Hans de Goede hdegoede@redhat.com wrote:
Hi,
On 06-01-18 20:56, Alexander Kapshuk wrote:
On Sat, Jan 6, 2018 at 9:43 PM, Hans de Goede hdegoede@redhat.com wrote:
HI,
On 06-01-18 20:30, Alexander Kapshuk wrote:
On Sat, Jan 6, 2018 at 6:00 PM, Hans de Goede hdegoede@redhat.com wrote:
Hi,
On 06-01-18 15:20, Alexander Kapshuk wrote:
On Mon, Dec 25, 2017 at 04:42:59PM +0200, Alexander Kapshuk wrote: > > > > Sparse emitted the following warning: > ../drivers/staging/vboxvideo/vbox_fb.c:173:27: warning: incorrect > type > in > assignment (different address spaces) > ../drivers/staging/vboxvideo/vbox_fb.c:173:27: expected char > [noderef] > asn:2*screen_base > ../drivers/staging/vboxvideo/vbox_fb.c:173:27: got void *virtual > > The vbox_bo buffer object kernel mapping is handled by a call > to ttm_bo_kmap() prior to the assignment of bo->kmap.virtual to > info->screen_base of type char __iomem*. > Casting bo->kmap.virtual to char __iomem* in this assignment fixes > the warning. > > vboxvideo: Fix address space of expression removal sparse warning > > Sparse emitted the following warning: > ../drivers/staging/vboxvideo/vbox_main.c:64:25: warning: cast removes > address space of expression > > vbox->vbva_buffers iomapping is handled by calling vbox_accel_init() > from vbox_hw_init(). > __force attribute is used in assignment to vbva to fix the warning. > > Signed-off-by: Alexander Kapshuk alexander.kapshuk@gmail.com > --- > drivers/staging/vboxvideo/vbox_fb.c | 2 +- > drivers/staging/vboxvideo/vbox_main.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/vboxvideo/vbox_fb.c > b/drivers/staging/vboxvideo/vbox_fb.c > index 8aed248db6e2..43c39eca4ae1 100644 > --- a/drivers/staging/vboxvideo/vbox_fb.c > +++ b/drivers/staging/vboxvideo/vbox_fb.c > @@ -170,7 +170,7 @@ static int vboxfb_create(struct drm_fb_helper > *helper, > drm_fb_helper_fill_var(info, &fbdev->helper, > sizes->fb_width, > sizes->fb_height); > > - info->screen_base = bo->kmap.virtual; > + info->screen_base = (char __iomem *)bo->kmap.virtual; > info->screen_size = size; > > #ifdef CONFIG_DRM_KMS_FB_HELPER
This fix looks good to me.
> diff --git a/drivers/staging/vboxvideo/vbox_main.c > b/drivers/staging/vboxvideo/vbox_main.c > index 80bd039fa08e..973b3bcc04b1 100644 > --- a/drivers/staging/vboxvideo/vbox_main.c > +++ b/drivers/staging/vboxvideo/vbox_main.c > @@ -61,7 +61,7 @@ void vbox_enable_accel(struct vbox_private *vbox) > if (vbox->vbva_info[i].vbva) > continue; > > - vbva = (void *)vbox->vbva_buffers + i * > VBVA_MIN_BUFFER_SIZE; > + vbva = (void __force *)vbox->vbva_buffers + i * > VBVA_MIN_BUFFER_SIZE; > if (!vbva_enable(&vbox->vbva_info[i], > vbox->guest_pool, vbva, i)) { > /* very old host or driver error. */
Hmm, isn't there a cleaner way to fix this ? Maybe make vbva_enable's argument (and the local vbva variable) of type u8 __iomem * too ?
Regards,
Hans
Hi Hans,
Thanks for your prompt response.
I had a good look at the vbva_enable() function's definition and to the best of my knowledge, changing vbva's type from 'struct vbva_buffer *' to 'u8 __iomem *' wouldn't work. vbva_enable() relies on vbva's type being a pointer to 'struct vbva_buffer': vbva's memory gets set to zero; some of vbva's members are initialised to particular values; vbva_ctx->vbva expects vbva to be a pointer to 'struct vbva_buffer' as well;
Or am I misreading this?
No your not misreading this I did not check the function myself before commenting myself, my bad.
Thanks for confirming my understanding of this.
What are your thoughts on this?
Lets just move ahead with your original fix:
Did you want me to resend the patch, or is someone going to pull the original one I sent into their tree? Thanks.
I expect Greg to pick it up without a resend. But with all the Spectre stuff going on right now he might miss it, so I would say give it 14 days and if he has not picked it up by then, resend it with my reviewed-by added.
Regards,
Hans
Understood. Thanks.
dri-devel@lists.freedesktop.org