Still think we should just replace this whole thing with:

if (blkio->Media->BlockSize != 512 || blkio->Media->BlockSize != 4096)

This is not as simple as that.

UEFI 2.7 does state about BlockSize:
The intrinsic block size of the device. If the media changes, then this field is updated.Returns the number of bytes per logical block. For ATA devices, this is reported in IDENTIFY DEVICE data words 117-118 (i.e., Words per Logical Sector) (see ATA8-ACS). For SCSI devices, this is reported in the READ CAPACITY (16) parameter data Logical Block Length In Bytes field (see SBC-3).

Now ATA IDENTIFY DEVICE:
7.18.7.61 Words 117..118: Logical sector size
Words 117..118 are a DWord field that indicates the size of device logical sectors in words. The value of logical sector size shall be equal to or greater than 256. The value of logical sector size shall be valid when bit 12 of word 106 is set to one. All logical sectors on a device shall be this length. When bit 12 of word 106 is cleared to zero words 117..118 shall be cleared to zero.

So from ATA, we have at least 256 bytes.

and SCSI READ CAPACITY:
The BLOCK LENGTH IN BYTES field contains the number of bytes of user data in the logical block indicated by the RETURNED LOGICAL BLOCK ADDRESS field. This value does not include protection information or additional information (e.g., ECC bytes) recorded on the medium.

For iPXE case I did confirm the stub (src/interface/efi/efi_file.c in ipxe source), they do implement FILE interface and to work around other issues, they indeed do insert the dummy block io into the stack, so this test is valid and does fix the issue.

I am still a bit hesitant about validating the BlockSize for other values, and mostly because it may help to identify some weird issues if those should arise.

Does our code work with 256 sector size? No. I doubt it works for < 512 for various reasons (not least that we read a sector, then access data > 256 unconditionally). Just because an ATA might possibly be a weird number doesn't mean we should accept it here. We can't use it anyway, and I doubt there's been more than a half a dozen drives made with 256 byte sectors, all in the early days of ATA.

I'm more inclined to filter all bogus values. If we know the code can't possibly work with odd-ball sector sizes, we should fail early and tell people they lose. If it's important, they'll let us know. It's been my experience in the past that the harder I tried to support oddball configurations, especially ones I could only lightly test, the result was rarely that the oddballs just worked and more often that the attempt interfered with normal device support.

For better or worse, 512, 4k and soon 64k are the only sector sizes that we'll see in the wild.

I don't think this is a power of 2 test, but rather just an even / odd test.

#define ispow2(x) ((x) != 0 && !((x) & ((x) - 1)))

is the canonical one-liner...

#define powerof2(x) ((((x)-1)&(x))==0)

can be found in sys/param.h and could be used. It falsely identifies 0 as a power of 2, but that's OK for this purpose.

indeed, I obviously was way too sleepy already:D