On Tue, 12 May 2026 at 07:30, Philippe Mathieu-Daudé <[email protected]> wrote:
>
> From: Chad Jablonski <[email protected]>
>
> This fixes three bugs with the ati_set_dirty address calculation.
>
> First, vbe_start_addr is a word offset. All other values in the
> calculation are byte offsets. It must be converted to bytes.
>
> Second, when setting the dirty region with memory_region_set_dirty
> the vbe_start_addr is used to calculate the start of the dirty region.
> This is a problem because the vbe_start_addr is the offset at which scan out
> begins. This puts it in the visible screen coordinate system. The dirty
> region however is in the virtual screen coordinate system. This can cause both
> overmarking and missed updates. This is removed from the calculation.
>
> Third, when the start address of a blit is outside of the bounds check
> the entire blit is missed and not set to dirty. This happens even if the
> blit does partially overlap with the visible screen. The fix here is to
> find the intersection of the visible screen and the blit and mark only
> that region as dirty.
>
> This does not attempt to apply clipping to the blit. So there will be
> overmarking in some cases.
>
> Signed-off-by: Chad Jablonski <[email protected]>
> [balaton: drop excess parenthesis, use offsets instead of pointers]
> Reviewed-by: BALATON Zoltan <[email protected]>
> Tested-by: BALATON Zoltan <[email protected]>
> Signed-off-by: BALATON Zoltan <[email protected]>
> Message-ID: <[email protected]>
> Signed-off-by: Philippe Mathieu-Daudé <[email protected]>
Hi; Coverity warns here about some "we did a 32 x 32 multiply
and put the result into a 64 bit calculation" issue here (CID 1660048):
> ---
> hw/display/ati_2d.c | 24 +++++++++++++++++-------
> 1 file changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/hw/display/ati_2d.c b/hw/display/ati_2d.c
> index 504d1c57085..48498677c7e 100644
> --- a/hw/display/ati_2d.c
> +++ b/hw/display/ati_2d.c
> @@ -69,18 +69,28 @@ typedef struct {
> static void ati_set_dirty(VGACommonState *vga, const ATI2DCtx *ctx)
> {
> DisplaySurface *ds = qemu_console_surface(vga->con);
> + unsigned int bypp = ctx->bpp / 8;
> + hwaddr dirty_start = ctx->dst_offset + ctx->dst.x * bypp +
> + ctx->dst.y * ctx->dst_stride;
dst.y is uint16_t and dst_stride is int, so we do the multiply
at 32-bits, and then the whole set of additions at 32-bits before
putting it into a 64-bit hwaddr.
Should we cast one of them to 64-bits to avoid the overflow,
or do we know here that these values can't actually be large
enough to overflow ?
thanks
-- PMM
