Re: [PATCH 1/1] Fix PCIe LER when GRUB2 accesses non-enabled MMIO data from VGA

[Daniel Kiper]

On Thu, Mar 29, 2018 at 05:00:56PM +0200, Hans de Goede wrote:
> Hi,
>
> On 29-03-18 16:27, Mike Travis wrote:
> >
> >
> >On 3/29/2018 2:36 AM, Daniel Kiper wrote:
> >>On Thu, Mar 29, 2018 at 05:29:01PM +0800, Michael Chang wrote:
> >>>On Thu, Mar 29, 2018 at 11:02:51AM +0200, Daniel Kiper wrote:
> >>>>On Wed, Mar 28, 2018 at 11:42:18AM -0500, address@hidden wrote:
> >>>>>A GPU inserted into a PCIe I/O slot disappears during system startup.
> >>>>>The problem centers around GRUB and a specific VGA init function in
> >>>>>efi_uga.c.?? This causes an LER (link error recorvery) because the MMIO
> >>>>>memory has not been enabled before attempting access.
> >>>>>
> >>>>>The fix is to add the same coding used in other VGA drivers,
> >>>>>specifically
> >>>>>to add a check to insure that it is indeed a VGA controller.?? And then
> >>>>>enable the MMIO address space with the specific bits.
> >>>>>
> >>>>>Signed-off-by: Mike Travis <address@hidden>
> >>>>>Reviewed-by: Michael Chang <address@hidden>
> >>>>>Reviewed-by: Daniel Kiper <address@hidden>
> >>>>
> >>>>Well, please do not add somebody RB tag if he/she did not explicitly
> >>>>asked you to do that. And even in that case I was not able to look at
> >>>>this patch in advance. So, my RB should not be here. Additionally, in
> >>>>this situation I would like to ask if Michael approved his RB?
> >>>
> >>>We did have discussion about the patch before it was submitted upstream
> >>>but I
> >>>did not ask for RB as well.
> >>>
> >>>>Anyway, patch LGTM except one nitpick. I will apply the patch, in a
> >>>>week or
> >>>>so, with Michael's RB if I get confirmation that he approved it earlier.
> >>>
> >>>As I did not ask for it, it has to be removed.
> >>
> >>OK, I will commit this patch without your RB.
> >>
> >>Daniel
> >>
> >
> >Sorry, I was not aware of this custom.?? I thought that if someone
> >reviewed the code then that was an indication that it should be noted. I
> >am aware of the more restrictive Acked-by which is normally sent by the
> >"acker" to the submitter.?? I will be more mindful of this in the
> >future.?? (This is my first posting to any of the GNU mail lists).

No problem.

> Reviewed-by actually is stronger then Acked-by at least that is
> what I believe, I've always interpreted these 2 as:
>
> Acked-by: foo:     Foo took a quick glance at the proposed changes and
> thinks this is a sensible change
> Reviewed-by: foo:  Foo did a detailed review of the code and did not find
> any issues

I think that you can take look at Documentation/process/submitting-patches.rst
in Linux kernel source. Most of us more or less adhere to this. Well, I think
that I should put something similar into GRUB2 tree.

Daniel