Re: [hatari-devel] Patch: IDE support for sector sizes > 512 bytes

[Thomas Huth <th.huth@xxxxxxxxx>]

Am Sun, 11 Nov 2018 08:10:19 +0100
schrieb Thomas Huth <th.huth@xxxxxxxxx>:

> Am Fri, 9 Nov 2018 08:34:45 +0100
> schrieb Uwe Seimet <Uwe.Seimet@xxxxxxxxx>:
> [...]
> > > @@ -1802,7 +1802,6 @@
> > >  		uint64_t total_sectors;
> > >  
> > >  		bdrv_get_geometry(s->bs, &total_sectors);
> > > -		total_sectors >>= 2;
> > >  		if (total_sectors == 0)
> > >  		{
> > >  			ide_atapi_cmd_error(s, SENSE_NOT_READY,
> > > 
> > > Why this change? Even current QEMU still contains the shift, so I
> > > somehow doubt that this is really right?  
> > 
> > The sector size for emulated (data) CDs is 2048 bytes, and QEMU also
> > uses this size a few lines below:
> > 
> > 		cpu_to_ube32(buf + 4, 2048);
> > 
> > But if you shift to the right the calculated capacity is just 1/4 of
> > what it actually is. You can also see in HDDRUTIL that the result of
> > READ CAPACITY is just 1/4 of the actual file size when the shift is
> > applied. This looks like a bug in the QEMU code to me. You may want
> > to double-check.  
> 
> Ok, I see your point - but if it's wrong in that case, it's also wrong
> for the other cases (e.g. GPCMD_SEEK and GPCMD_READ_CDVD_CAPACITY)...
> 
> And since current QEMU (file hw/ide/atapi.c) also still contains these
> shifts, I'd like to get this discussed with the folks from QEMU first
> before we blindly fix this in the wrong way in Hatari only... I'll try
> to remember to ask this on the qemu-devel mailing list next week.

Ok, it took a little bit longer (sorry!), but I've now finally checked
the QEMU sources more closely to see whether it's also wrong there.
It's not. QEMU's bdrv_get_geometry() function is simply always
calculating with 512 byte sectors, no matter whether it's an CD-ROM
drive or a normal hard disk. That's why they've got the "total_sectors
>>= 2" shifts there. But since we're calculating with 2048 byte sectors
in Hatari, we can remove these shifts indeed.

I've pushed a fix to the repository now. Uwe, could you please check
whether it's working as expected?

 Thanks,
  Thomas