Hi,

Ok, you are right.
On Jun 12, 2015 7:57 PM, "Andrei Borzenkov" <arvidj...@gmail.com> wrote:

> В Fri, 12 Jun 2015 19:52:31 +0800
> Heiher <r...@hev.cc> пишет:
>
> > Hi,
> >
> > Thanks for reivew.
> >
> > On Fri, Jun 12, 2015 at 6:02 PM, Andrei Borzenkov <arvidj...@gmail.com>
> > wrote:
> >
> > > В Mon, 8 Jun 2015 17:25:16 +0800
> > > Heiher <r...@hev.cc> пишет:
> > >
> > > > --- a/grub-core/bus/pci.c
> > > > +++ b/grub-core/bus/pci.c
> > > > @@ -72,7 +72,7 @@ grub_dma_get_virt (struct grub_pci_dma_chunk *ch)
> > > >  grub_uint32_t
> > > >  grub_dma_get_phys (struct grub_pci_dma_chunk *ch)
> > > >  {
> > > > -  return (((grub_uint32_t) ch) & 0x1fffffff) | 0x80000000;
> > > > +  return (((grub_uint32_t) ch) & 0x1fffffff);
> > >
> > > I do not have experience with hardware but that does not look right.
> > > Either this chunk breaks existing hardware or this part was not needed
> > > in the first place. I suspect the former because later you re-add the
> > > same code in subsequent patch.
> > >
> >
> > I think this is a old mistake that caused grub on loongson3 broken. The
> dma
> > chunk address (ch & 0x1fffffff) is physical address. and it or 0x80000000
> > is cached virtual address.
> >
>
> So if this is not needed for existing platforms please make it separate
> patch to make it obvious and facilitate regression search. Do not hide
> it inside additional platform support.
>
> > My next subsequent patch fixed grub_dma_get_virt (not grub_dma_get_phys)
> > function,  because the Loongson 3B has a dma cached coherent issue, just
> > only access memory by uncached for dma. so i add a grub_dma_coherent flag
> > to control grub_dma_get_virt return cached virtual address or uncached.
> >
> >
> > >
> > > This makes it hard to bisect later as it leaves some commits in
> > > non-functional state.
> > >
> > > Could you please rearrange your patches so that they do not impact
> > > existing platforms?
> > >
> >
> > I have tested on Loongson 2F and Loongson 3A, 3B platforms. I have not
> > tested on qemu mips, and i will do that next. thanks!
> >
>
>
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Reply via email to