On Wed, Feb 02, 2022 at 11:50:50AM +0100, Alexander Graf wrote: > > On 02.02.22 02:52, Kevin O'Connor wrote: > > 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. > > > Looking at the spec, lbads is a full well aligned 8 bits. The collision here > is between rp and res, with rp taking only 2 bits and res the remaining 6.
Ah, I misread the spec. So, the original patch removing the "res" struct member should be fine. > Is using bitfields acceptable in SeaBIOS? If so, the patch below should > automatically give us masking as well. I'd recommend against using C bitfields to access hardware registers. -Kevin _______________________________________________ SeaBIOS mailing list -- seabios@seabios.org To unsubscribe send an email to seabios-le...@seabios.org
