Re: CVS commit: src/sys/dev

[Rin Okuyama <rokuyama.rk%gmail.com@localhost>]

Hi Michael, thanks for kind review!

On 2025/04/12 19:01, Michael van Elst wrote:
> On Sat, Apr 12, 2025 at 06:24:44PM +0900, Rin Okuyama wrote:
> > Hi!
> >
> > I've made a draft patch to support dumping against > 2Gi blocks
> > for backends like nvme(4) or virtio(4).
> >
> > Does it look reasonable to you?
>
>
>
> Hi rin,
>
>
> > -	if (blkno < 0 || blkno + nblk - 1 > INT_MAX)
> > +	if (blkno < 0 ||
> > +	    ((sc->sc_flags & LDF_BLOCK32) != 0 && blkno + nblk > UINT32_MAX))
> >   		return (EIO);
>
>
> I think the check for the upper bound needs to be like I wrote it so
> that the last block written doesn't exceed the maximum signed int value.

Sorry, I should have mentioned this part in the previous message.
Thank you again for your careful check.

Some H/W seems to accept uint32_t address, but, yes, you are right.
Safer is better; I cannot check every datasheet.

IMO, LDF_BLOCK32 flag bit should mean H/W capability, not software
restriction. I will remove the flag, and move the second check into
each driver.

Off-by-one was introduced by my thinko ;)

PS
For ld_sdmmc.c, IIUC, check for ld_sdmmc_dump() yields

```
if (blkno + blkcnt - 1 > sc->sc_sf->csd.capacity)
	return EIO;
```

Is this correct?

If so, shouldn't we take into account b_bcount for ld_sdmmc_dobio()?
https://nxr.netbsd.org/xref/src/sys/dev/sdmmc/ld_sdmmc.c#458

Thanks,
rin

> Otherwise it looks ok.
>
> The current version (check only) could be pulled up. The changed
> backend ABI would require more compat code.
>
>
> Greetings,