Re: [Xen-devel] [PATCH 1/2] x86/boot: fix early error display
>>> On 13.10.17 at 11:51, wrote: > On 12/10/17 21:55, Andrew Cooper wrote: >> On 12/10/2017 21:50, Doug Goldstein wrote: >>> From: David Esler >>> >>> In 9180f5365524 a change was made to the send_chr function to take in >>> C-strings and print out a character at a time until a NULL was >>> encountered. However there is no code to increment the current character >>> position resulting in an endless loop of the first character. This adds >>> a simple increment. >>> >>> Reviewed-by: Doug Goldstein >>> Signed-off-by: David Esler >> >> Reviewed-by: Andrew Cooper >> >> CC'ing Julien as release manager. This should be included in 4.10 IMO, >> and is a backport candidate. > > I agree. > > Release-acked-by: Julien Grall So I take this is a pre-ack to the eventual patch with the bug fixed that Daniel has pointed out? Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 1/2] x86/boot: fix early error display
Hi Andrew, On 12/10/17 21:55, Andrew Cooper wrote: On 12/10/2017 21:50, Doug Goldstein wrote: From: David Esler In 9180f5365524 a change was made to the send_chr function to take in C-strings and print out a character at a time until a NULL was encountered. However there is no code to increment the current character position resulting in an endless loop of the first character. This adds a simple increment. Reviewed-by: Doug Goldstein Signed-off-by: David Esler Reviewed-by: Andrew Cooper CC'ing Julien as release manager. This should be included in 4.10 IMO, and is a backport candidate. I agree. Release-acked-by: Julien Grall Cheers, --- xen/arch/x86/boot/head.S | 1 + 1 file changed, 1 insertion(+) diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S index fd6fc337fe..f48bbbd2e5 100644 --- a/xen/arch/x86/boot/head.S +++ b/xen/arch/x86/boot/head.S @@ -174,6 +174,7 @@ not_multiboot: mov sym_esi(vga_text_buffer),%edi .Lsend_chr: mov (%esi),%bl +inc %esi test%bl,%bl# Terminate on '\0' sentinel je .Lhalt mov $0x3f8+5,%dx # UART Line Status Register ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 1/2] x86/boot: fix early error display
> On Oct 12, 2017, at 4:27 PM, Daniel Kiper wrote: > >> On Thu, Oct 12, 2017 at 03:50:06PM -0500, Doug Goldstein wrote: >> From: David Esler >> >> In 9180f5365524 a change was made to the send_chr function to take in >> C-strings and print out a character at a time until a NULL was >> encountered. However there is no code to increment the current character >> position resulting in an endless loop of the first character. This adds >> a simple increment. >> >> Reviewed-by: Doug Goldstein >> Signed-off-by: David Esler >> --- >> xen/arch/x86/boot/head.S | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S >> index fd6fc337fe..f48bbbd2e5 100644 >> --- a/xen/arch/x86/boot/head.S >> +++ b/xen/arch/x86/boot/head.S >> @@ -174,6 +174,7 @@ not_multiboot: >> mov sym_esi(vga_text_buffer),%edi >> .Lsend_chr: >> mov (%esi),%bl >> +inc %esi >> test%bl,%bl# Terminate on '\0' sentinel >> je .Lhalt >> mov $0x3f8+5,%dx # UART Line Status Register > > I have a feeling that you have tested this on machine without > VGA text buffer available. Then your fix works. However, if VGA > text buffer is available then %esi is increased twice. First time > by inc here and once again by movsb below. So, I think that the > issue have to be fixed in a bit different way. > > Daniel Correct. It was an EFI machine with serial only where I saw it in action. David has put together a new change and I’ll get it submitted tomorrow. — Doug ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 1/2] x86/boot: fix early error display
On Thu, Oct 12, 2017 at 03:50:06PM -0500, Doug Goldstein wrote: > From: David Esler > > In 9180f5365524 a change was made to the send_chr function to take in > C-strings and print out a character at a time until a NULL was > encountered. However there is no code to increment the current character > position resulting in an endless loop of the first character. This adds > a simple increment. > > Reviewed-by: Doug Goldstein > Signed-off-by: David Esler > --- > xen/arch/x86/boot/head.S | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S > index fd6fc337fe..f48bbbd2e5 100644 > --- a/xen/arch/x86/boot/head.S > +++ b/xen/arch/x86/boot/head.S > @@ -174,6 +174,7 @@ not_multiboot: > mov sym_esi(vga_text_buffer),%edi > .Lsend_chr: > mov (%esi),%bl > +inc %esi > test%bl,%bl# Terminate on '\0' sentinel > je .Lhalt > mov $0x3f8+5,%dx # UART Line Status Register I have a feeling that you have tested this on machine without VGA text buffer available. Then your fix works. However, if VGA text buffer is available then %esi is increased twice. First time by inc here and once again by movsb below. So, I think that the issue have to be fixed in a bit different way. Daniel ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 1/2] x86/boot: fix early error display
On 12/10/2017 21:50, Doug Goldstein wrote: > From: David Esler > > In 9180f5365524 a change was made to the send_chr function to take in > C-strings and print out a character at a time until a NULL was > encountered. However there is no code to increment the current character > position resulting in an endless loop of the first character. This adds > a simple increment. > > Reviewed-by: Doug Goldstein > Signed-off-by: David Esler Reviewed-by: Andrew Cooper CC'ing Julien as release manager. This should be included in 4.10 IMO, and is a backport candidate. > --- > xen/arch/x86/boot/head.S | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S > index fd6fc337fe..f48bbbd2e5 100644 > --- a/xen/arch/x86/boot/head.S > +++ b/xen/arch/x86/boot/head.S > @@ -174,6 +174,7 @@ not_multiboot: > mov sym_esi(vga_text_buffer),%edi > .Lsend_chr: > mov (%esi),%bl > +inc %esi > test%bl,%bl# Terminate on '\0' sentinel > je .Lhalt > mov $0x3f8+5,%dx # UART Line Status Register ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
