On Tue, Feb 01, 2022 at 08:39:10PM +0100, Florian Larysch wrote: > On Thu, Jan 27, 2022 at 11:37:52AM -0500, Kevin O'Connor wrote: > > Thanks. I don't know enough about NVMe to review this patch though. > > Maybe Julian or Alex could comment? > > Happy to hear their comments on this. However, I can also try to explain > the reasoning in a bit more detail. > > This follows from the NVMe spec[1]: The Identify command returns a data > structure that contains packed LBAF records (Figure 249, starting at > offset 131). This is represented by struct nvme_identify_ns in the > SeaBIOS code. > > Figure 250 gives the structure of these records and this is where the > aforementioned discrepancy lies.
Thanks. I agree that this should be fixed in SeaBIOS. However, your patch removes the "reserved" field, which doesn't look correct. It sounds like the "res" struct member should remain, but the "lbads" and "rp" members should be turned into a new "lbads_rp" member. Also, the nvme.c code should be updated so that the read of lbads performs the proper bit masking. -Kevin > > I think what happened was that somebody mistook the reserved 6 uppermost > bits as a whole byte and put it into struct nvme_lba_format as such. To > correctly parse the RP field, you'd still need to mask off these bits, > but the code only uses the LBADS/MS fields anyway. > > I encountered this on an NVMe device that had FLBAS[3:0] != 0, which > caused nonsensical outputs of the various dprintf()s and also throws off > everything that depends on knowing the block size. Empirically, the > patch fixes these problems. > > [1] > https://nvmexpress.org/wp-content/uploads/NVM-Express-1_4c-2021.06.28-Ratified.pdf _______________________________________________ SeaBIOS mailing list -- seabios@seabios.org To unsubscribe send an email to seabios-le...@seabios.org
