Re: [PATCH 1/2] disk: Add support for device-specific malloc function

[Leif Lindholm]

On Mon, Feb 22, 2016 at 07:56:25PM +0300, Andrei Borzenkov wrote:
> 22.02.2016 17:02, Leif Lindholm пишет:
> > On Mon, Feb 22, 2016 at 10:57:24AM +0300, Andrei Borzenkov wrote:
> >> 19.02.2016 19:18, Leif Lindholm пишет:
> >>> Some disk types have allocation requirements beyond normal grub_malloc.
> >>> Add a function pointer to grub_disk_t and a wrapper function in
> >>> kern/disk.c making use of that function if available, to enable these
> >>> disk drivers to implement their own malloc.
> >>
> >> The problem is not (only) grub_disk_read_small(), but this part in
> >> grub_disk_read:
> >>
> >>       if (agglomerate)
> >>         {
> >>           grub_disk_addr_t i;
> >>
> >>           err = (disk->dev->read) (disk, transform_sector (disk, sector),
> >>                                    agglomerate << (GRUB_DISK_CACHE_BITS
> >>                                                    + GRUB_DISK_SECTOR_BITS
> >>                                                    - 
> >> disk->log_sector_size),
> >>                                    buf);
> >>
> >> which reads directly into user supplied buffer. May be we can allocate
> >> contiguous cache block here but put pointers to multiple chunks inside
> >> it. Possible implementation is to have second layer of reference counted
> >> memory blocks with cache entries containing pointer + offset into them.
> > 
> > Whoops!
> > 
> > Understood.
> > 
> > So how about merging the two concepts?
> > Including a patch to go with (after) the previous two to catch any
> > remaining unaligned accesses in grub_efidisk_readwrite().
> > With this applied, I get no fixups from a normal Linux boot (linux +
> > initrd), but see them when exploring filesystems from the command
> > line.
> > 
> > Whilst a bit clunky, this seems much short-term preferable to going
> > back and redesigning the disk subsystem to understand that alignment
> > matters. Although given the number of exceptions we seem to be
> > amassing, that does not sound like a bad idea for future.
> > 
> 
> Could you test attached patch with your alignment fixes on top. This
> implements my idea of using shared buffers. Seems to work in naive testing.

Testing this with a grub-mkstandalone image, I get:

kern/dl.c:556: flushing 0x10f1 bytes at 0x9ffb5ac20
kern/dl.c:649: module name: tar
kern/dl.c:650: init function: 0x9ffb5b220
kern/disk.c:217: Opening `memdisk'...
kern/fs.c:56: Detecting tarfs...

And then spectacular crash in UEFI due to an EL2 translation fault.

Regards,

Leif