headers
Scott Duplichan | 2 May 2011 19:48
Favicon

Re: non-emulated AHCI hardware

Kevin O'Connor wrote:

]Yeah.  One can call malloc_low() during the init phase to allocate
]this buffer.  Ideally, though, there would be some way to share buffer
]space with the other users (eg, cdemu, ide dma, usb).
]
]-Kevin

Hello Kevin,

The attached patch used a malloc_low disk sector buffer when the
caller's buffer is not aligned. The read handling is tested with
DOS and the write handling is untested.

There is another problem. The same DOS image as eltorito CD-ROM
causes the same problem, but with atapi. So maybe a shared 2048
byte buffer is needed.

Thanks,
Scott
Attachment (ahci-002.patch): application/octet-stream, 7356 bytes
_______________________________________________
SeaBIOS mailing list
SeaBIOS <at> seabios.org
http://www.seabios.org/mailman/listinfo/seabios
Permalink | Reply |
headers
Kevin O'Connor | 11 May 2011 04:14

Re: non-emulated AHCI hardware

On Mon, May 02, 2011 at 12:48:45PM -0500, Scott Duplichan wrote:
> Kevin O'Connor wrote:
> ]Yeah.  One can call malloc_low() during the init phase to allocate
> ]this buffer.  Ideally, though, there would be some way to share buffer
> ]space with the other users (eg, cdemu, ide dma, usb).
> ]
> ]-Kevin
> 
> Hello Kevin,
> 
> The attached patch used a malloc_low disk sector buffer when the
> caller's buffer is not aligned. The read handling is tested with
> DOS and the write handling is untested.
> 
> There is another problem. The same DOS image as eltorito CD-ROM
> causes the same problem, but with atapi. So maybe a shared 2048
> byte buffer is needed.

Thanks Scott,

I'd like to commit these fixes.  Is there any chance you could break
them up into separate patches - one per fix?

Gerd - can you also take a look through Scott's changes and confirm
they look okay to you?

-Kevin
Permalink | Reply |
headers
Gerd Hoffmann | 25 May 2011 12:05
Picon
Favicon

Re: non-emulated AHCI hardware

> Thanks Scott,
>
> I'd like to commit these fixes.  Is there any chance you could break
> them up into separate patches - one per fix?

That would be great for review too.

thanks,
   Gerd
Permalink | Reply |
headers
Scott Duplichan | 25 May 2011 16:38
Favicon

Re: non-emulated AHCI hardware

Gerd Hoffmann wrote,

]> Thanks Scott,
]>
]> I'd like to commit these fixes.  Is there any chance you could break
]> them up into separate patches - one per fix?
]
]That would be great for review too.
]
]thanks,
]   Gerd

Hello Gerd,

Oh, I see now I completely overlooked Kevin's original request to
break up the AHCI patch. I will do that later today.

Thanks,
Scott
Permalink | Reply |
headers
Scott Duplichan | 26 May 2011 07:07
Favicon

Re: non-emulated AHCI hardware

Gerd Hoffmann wrote,

]> Thanks Scott,
]>
]> I'd like to commit these fixes.  Is there any chance you could break
]> them up into separate patches - one per fix?
]
]That would be great for review too.
]
]thanks,
]   Gerd

Hello Gerd,

Attached is a two part version of the previous patch. Stage1 is enough
to get through most OS installs. Stage2 adds unaligned buffer support
needed for MS-DOS. 

Thanks,
Scott
Attachment (stage2.patch): application/octet-stream, 2781 bytes
Attachment (stage1.patch): application/octet-stream, 5275 bytes
_______________________________________________
SeaBIOS mailing list
SeaBIOS <at> seabios.org
http://www.seabios.org/mailman/listinfo/seabios
Permalink | Reply |
headers
Gerd Hoffmann | 26 May 2011 14:00
Picon
Favicon

Re: non-emulated AHCI hardware

   Hi,

> Attached is a two part version of the previous patch. Stage1 is enough
> to get through most OS installs. Stage2 adds unaligned buffer support
> needed for MS-DOS.

Hmm, patch #1 is still a collection of multiple changes, looks a bit 
like trying this and that until it somehow worked, without figuring 
which changes where needed to make the box boot.  Also the changelog 
should describe what was changed and why, not only the effect of the change.

Polling the IRQ status looks like a sensible thing to do.  Note that 
there might be multiple bits set in the IRQ status register, so you 
can't use irqbits == 0x01 to check the status.  qemu fails to boot 
because of that bug.  Also it isn't obvious why you are looking for 
others than the "Device to Host Register FIS" interrupt.  Care to explain?

I've attached a patch which switches ahci over to irq status polling, 
based on your patch.  Works nicely in qemu.  Can you give it a spin on 
real hardware?

cheers,
   Gerd

From 4be00033adb966eb40c1ca2700cc770db0ff682a Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel <at> redhat.com>
Date: Thu, 26 May 2011 13:39:26 +0200
Subject: [PATCH] ahci: use interrupt status register
(Continue reading)

Permalink | Reply |
headers
Scott Duplichan | 26 May 2011 20:09
Favicon

Re: non-emulated AHCI hardware

Gerd Hoffmann wrote:

]   Hi,
]
]> Attached is a two part version of the previous patch. Stage1 is enough
]> to get through most OS installs. Stage2 adds unaligned buffer support
]> needed for MS-DOS.
]
]Hmm, patch #1 is still a collection of multiple changes, looks a bit 
]like trying this and that until it somehow worked, without figuring 
]which changes where needed to make the box boot.

Thank you for the feedback. This is a pretty harsh assessment of work
that transforms non-functioning code into something usable on real hardware.
Have you ever written code for real hardware or do work strictly with
software models?

]  Also the changelog 
]should describe what was changed and why, not only the effect of the
]change.

These documents will help you understand the changes:
http://download.intel.com/technology/serialata/pdf/rev1_3.pdf
http://www.lttconn.com/res/lttconn/pdres/201005/20100521170123066.pdf

]Polling the IRQ status looks like a sensible thing to do.  Note that 
]there might be multiple bits set in the IRQ status register, so you 
]can't use irqbits == 0x01 to check the status.  qemu fails to boot 
]because of that bug.
(Continue reading)

Permalink | Reply |
headers
Gerd Hoffmann | 27 May 2011 10:18
Picon
Favicon

Re: non-emulated AHCI hardware

On 05/26/11 20:09, Scott Duplichan wrote:
> Gerd Hoffmann wrote:
>
> ]   Hi,
> ]
> ]>  Attached is a two part version of the previous patch. Stage1 is enough
> ]>  to get through most OS installs. Stage2 adds unaligned buffer support
> ]>  needed for MS-DOS.
> ]
> ]Hmm, patch #1 is still a collection of multiple changes, looks a bit
> ]like trying this and that until it somehow worked, without figuring
> ]which changes where needed to make the box boot.
>
> Thank you for the feedback. This is a pretty harsh assessment of work
> that transforms non-functioning code into something usable on real hardware.

Well, it breaks on qemu, and it does because of bugs in the code.

> Have you ever written code for real hardware or do work strictly with
> software models?

I've actually done linux driver development (video4linux) before I've 
started working in the virtualization field.

> ]Polling the IRQ status looks like a sensible thing to do.  Note that
> ]there might be multiple bits set in the IRQ status register, so you
> ]can't use irqbits == 0x01 to check the status.  qemu fails to boot
> ]because of that bug.
>
> Have you considered the possibility that the reason this works on real
(Continue reading)

Permalink | Reply |

Gmane