Platform/RaspberryPi/RPi3/RPi3.dsc | 7 +++++-- Platform/RaspberryPi/RPi4/RPi4.dsc | 7 +++++-- 2 files changed, 10 insertions(+), 4 deletions(-)
Fix an ASSERT with the TFTP dynamic Shell command on the RPi3 and RPi4 when running DEBUG builds. Also, enable the command by default for all builds. Cc: Leif Lindholm <leif@nuviainc.com> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com> Cc: Pete Batard <pete@akeo.ie> Cc: Andrei Warkentin <awarkentin@vmware.com> Samer El-Haj-Mahmoud (4): Platform/RaspberryPi/RPi3: Fix TFTP dynamic command initialization Platform/RaspberryPi/RPi4: Fix TFTP dynamic command initialization Platform/RaspberryPi/RPi3: Enable TFTP command by default Platform/RaspberryPi/RPi4: Enable TFTP command by default Platform/RaspberryPi/RPi3/RPi3.dsc | 7 +++++-- Platform/RaspberryPi/RPi4/RPi4.dsc | 7 +++++-- 2 files changed, 10 insertions(+), 4 deletions(-) -- 2.17.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#57565): https://edk2.groups.io/g/devel/message/57565 Mute This Topic: https://groups.io/mt/73127191/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On 4/19/20 3:04 PM, Samer El-Haj-Mahmoud wrote: > Fix an ASSERT with the TFTP dynamic Shell command on the > RPi3 and RPi4 when running DEBUG builds. Also, enable the > command by default for all builds. > Fixing the ASSERT is fine but I am reluctant to enable this by default. It is a non-standard hack that ARM contributed in the past, and is not covered by the EFI of Shell specifications. If RPi4 is intended to be a showcase for UEFI on ARM done right, we should not enable this at all. > Cc: Leif Lindholm <leif@nuviainc.com> > Cc: Ard Biesheuvel <ard.biesheuvel@arm.com> > Cc: Pete Batard <pete@akeo.ie> > Cc: Andrei Warkentin <awarkentin@vmware.com> > > Samer El-Haj-Mahmoud (4): > Platform/RaspberryPi/RPi3: Fix TFTP dynamic command initialization > Platform/RaspberryPi/RPi4: Fix TFTP dynamic command initialization > Platform/RaspberryPi/RPi3: Enable TFTP command by default > Platform/RaspberryPi/RPi4: Enable TFTP command by default > > Platform/RaspberryPi/RPi3/RPi3.dsc | 7 +++++-- > Platform/RaspberryPi/RPi4/RPi4.dsc | 7 +++++-- > 2 files changed, 10 insertions(+), 4 deletions(-) > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#57570): https://edk2.groups.io/g/devel/message/57570 Mute This Topic: https://groups.io/mt/73127191/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
> -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ard > Biesheuvel via groups.io > Sent: Sunday, April 19, 2020 9:34 AM > To: Samer El-Haj-Mahmoud <samer@elhajmahmoud.com>; > devel@edk2.groups.io > Cc: Leif Lindholm <leif@nuviainc.com>; Pete Batard <pete@akeo.ie>; Andrei > Warkentin (awarkentin@vmware.com) <awarkentin@vmware.com> > Subject: Re: [edk2-devel] [edk2-platform][PATCH v1 0/4] Platform/RaspberryPi > : Enable TFTP shell command > > On 4/19/20 3:04 PM, Samer El-Haj-Mahmoud wrote: > > Fix an ASSERT with the TFTP dynamic Shell command on the > > RPi3 and RPi4 when running DEBUG builds. Also, enable the command by > > default for all builds. > > > > Fixing the ASSERT is fine but I am reluctant to enable this by default. > It is a non-standard hack that ARM contributed in the past, and is not covered > by the EFI of Shell specifications. If RPi4 is intended to be a showcase for UEFI > on ARM done right, we should not enable this at all. > That is OK. Are you fine just reviewing/pushing the PCD patches (and dropping the enable ones), or want me to send a new series without the enable patches ? > > > > > Cc: Leif Lindholm <leif@nuviainc.com> > > Cc: Ard Biesheuvel <ard.biesheuvel@arm.com> > > Cc: Pete Batard <pete@akeo.ie> > > Cc: Andrei Warkentin <awarkentin@vmware.com> > > > > Samer El-Haj-Mahmoud (4): > > Platform/RaspberryPi/RPi3: Fix TFTP dynamic command initialization > > Platform/RaspberryPi/RPi4: Fix TFTP dynamic command initialization > > Platform/RaspberryPi/RPi3: Enable TFTP command by default > > Platform/RaspberryPi/RPi4: Enable TFTP command by default > > > > Platform/RaspberryPi/RPi3/RPi3.dsc | 7 +++++-- > > Platform/RaspberryPi/RPi4/RPi4.dsc | 7 +++++-- > > 2 files changed, 10 insertions(+), 4 deletions(-) > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#57571): https://edk2.groups.io/g/devel/message/57571 Mute This Topic: https://groups.io/mt/73127191/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On 4/19/20 4:00 PM, Samer El-Haj-Mahmoud wrote: >> -----Original Message----- >> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ard >> Biesheuvel via groups.io >> Sent: Sunday, April 19, 2020 9:34 AM >> To: Samer El-Haj-Mahmoud <samer@elhajmahmoud.com>; >> devel@edk2.groups.io >> Cc: Leif Lindholm <leif@nuviainc.com>; Pete Batard <pete@akeo.ie>; Andrei >> Warkentin (awarkentin@vmware.com) <awarkentin@vmware.com> >> Subject: Re: [edk2-devel] [edk2-platform][PATCH v1 0/4] Platform/RaspberryPi >> : Enable TFTP shell command >> >> On 4/19/20 3:04 PM, Samer El-Haj-Mahmoud wrote: >>> Fix an ASSERT with the TFTP dynamic Shell command on the >>> RPi3 and RPi4 when running DEBUG builds. Also, enable the command by >>> default for all builds. >>> >> >> Fixing the ASSERT is fine but I am reluctant to enable this by default. >> It is a non-standard hack that ARM contributed in the past, and is not covered >> by the EFI of Shell specifications. If RPi4 is intended to be a showcase for UEFI >> on ARM done right, we should not enable this at all. >> > > That is OK. > > Are you fine just reviewing/pushing the PCD patches (and dropping the enable ones), or want me to send a new series without the enable patches ? > No that's fine, I'll just take the first two patches. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#57572): https://edk2.groups.io/g/devel/message/57572 Mute This Topic: https://groups.io/mt/73127191/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On 2020.04.19 14:33, Ard Biesheuvel wrote: > On 4/19/20 3:04 PM, Samer El-Haj-Mahmoud wrote: >> Fix an ASSERT with the TFTP dynamic Shell command on the >> RPi3 and RPi4 when running DEBUG builds. Also, enable the >> command by default for all builds. >> > > Fixing the ASSERT is fine but I am reluctant to enable this by default. I'm going to second this. To answer a question Samer was asking elsewhere, this is actually part of the reason why TFTP is not enabled in the DEBUG builds we produce at https://github.com/pftf/RPi4 (See build_firmware.sh), the reasoning being that if someone encounters an issue with RELEASE and we ask them to troubleshoot with the DEBUG artifact, we want to eliminate potential troublemakers when they try that. > It is a non-standard hack that ARM contributed in the past, and is not > covered by the EFI of Shell specifications. If RPi4 is intended to be a > showcase for UEFI on ARM done right, we should not enable this at all. Here I have to point out that RPi4 becoming a showcase because we intend to is not what we are pursuing (because if it was a matter of "willing" a showcase into existence, we would have picked a platform with a lot less quirks, more comprehensive documentation, and so on). Instead, we estimate that due to its price point and widespread availability, it *is* going to become a de facto showcase, whether everybody likes it or not. And that is the reason we want to treat is as a showcase where possible. Regards, /Pete -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#57573): https://edk2.groups.io/g/devel/message/57573 Mute This Topic: https://groups.io/mt/73127191/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Hi folks, If we have to choose abstract goodness over functionality, why wouldn't we choose functionality? Functionality that's part of Tiano? The real world doesn't care about the TFTP command being an "unsupported hack" or not. So there's Tiano-specific code here. Big deal? To rephrase differently, why would either Pi 4 developers or Pi 4 UEFI users pay the cost of Tiano carrying code that somehow isn't "legit enough" to be enabled? I mean here we are again, where what goes into the code is being dictated by some abstract ideology instead of technical reasons? A ________________________________ From: Pete Batard <pete@akeo.ie> Sent: Sunday, April 19, 2020 9:19 AM To: Ard Biesheuvel <ard.biesheuvel@arm.com>; Samer El-Haj-Mahmoud <samer@elhajmahmoud.com>; devel@edk2.groups.io <devel@edk2.groups.io> Cc: Leif Lindholm <leif@nuviainc.com>; Andrei Warkentin <awarkentin@vmware.com> Subject: Re: [edk2-platform][PATCH v1 0/4] Platform/RaspberryPi : Enable TFTP shell command On 2020.04.19 14:33, Ard Biesheuvel wrote: > On 4/19/20 3:04 PM, Samer El-Haj-Mahmoud wrote: >> Fix an ASSERT with the TFTP dynamic Shell command on the >> RPi3 and RPi4 when running DEBUG builds. Also, enable the >> command by default for all builds. >> > > Fixing the ASSERT is fine but I am reluctant to enable this by default. I'm going to second this. To answer a question Samer was asking elsewhere, this is actually part of the reason why TFTP is not enabled in the DEBUG builds we produce at https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fpftf%2FRPi4&data=02%7C01%7Cawarkentin%40vmware.com%7C27a7ee62d7734fc2000b08d7e46cb8be%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637229027923593475&sdata=rKm%2BRZE%2FPWkvNTinHgQ5qgA9EDcD4lyIZSZMsHf66d0%3D&reserved=0 (See build_firmware.sh), the reasoning being that if someone encounters an issue with RELEASE and we ask them to troubleshoot with the DEBUG artifact, we want to eliminate potential troublemakers when they try that. > It is a non-standard hack that ARM contributed in the past, and is not > covered by the EFI of Shell specifications. If RPi4 is intended to be a > showcase for UEFI on ARM done right, we should not enable this at all. Here I have to point out that RPi4 becoming a showcase because we intend to is not what we are pursuing (because if it was a matter of "willing" a showcase into existence, we would have picked a platform with a lot less quirks, more comprehensive documentation, and so on). Instead, we estimate that due to its price point and widespread availability, it *is* going to become a de facto showcase, whether everybody likes it or not. And that is the reason we want to treat is as a showcase where possible. Regards, /Pete -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#57574): https://edk2.groups.io/g/devel/message/57574 Mute This Topic: https://groups.io/mt/73127191/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Andrei, In case this is your concern, please note that we are not removing TFTP support at all, which is enabled for the RELEASE builds we produce and will remain so (and which anyone can enable with the macro if they wish). All that will be changed by the updated proposal is that the current DEBUG ASSERT will be fixed and TFTP support will remain optional, like it is today. So, in this case, I don't think your concern is warranted, because we're not actually taking any step to deprive anyone of any functionality they might wish for, and, even with the revised patch, TFTP will remain enabled in our RELEASE binaries, exactly as it has been before. Regards, /Pete On 2020.04.19 20:56, Andrei Warkentin wrote: > Hi folks, > > If we have to choose abstract goodness over functionality, why wouldn't > we choose functionality? Functionality that's part of Tiano? The real > world doesn't care about the TFTP command being an "unsupported hack" or > not. So there's Tiano-specific code here. Big deal? To rephrase > differently, why would either Pi 4 developers or Pi 4 UEFI users pay the > cost of Tiano carrying code that somehow isn't "legit enough" to be enabled? > > I mean here we are again, where what goes into the code is being > dictated by some abstract ideology instead of technical reasons? > > A > ------------------------------------------------------------------------ > *From:* Pete Batard <pete@akeo.ie> > *Sent:* Sunday, April 19, 2020 9:19 AM > *To:* Ard Biesheuvel <ard.biesheuvel@arm.com>; Samer El-Haj-Mahmoud > <samer@elhajmahmoud.com>; devel@edk2.groups.io <devel@edk2.groups.io> > *Cc:* Leif Lindholm <leif@nuviainc.com>; Andrei Warkentin > <awarkentin@vmware.com> > *Subject:* Re: [edk2-platform][PATCH v1 0/4] Platform/RaspberryPi : > Enable TFTP shell command > On 2020.04.19 14:33, Ard Biesheuvel wrote: >> On 4/19/20 3:04 PM, Samer El-Haj-Mahmoud wrote: >>> Fix an ASSERT with the TFTP dynamic Shell command on the >>> RPi3 and RPi4 when running DEBUG builds. Also, enable the >>> command by default for all builds. >>> >> >> Fixing the ASSERT is fine but I am reluctant to enable this by default. > > I'm going to second this. > > To answer a question Samer was asking elsewhere, this is actually part > of the reason why TFTP is not enabled in the DEBUG builds we produce at > https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fpftf%2FRPi4&data=02%7C01%7Cawarkentin%40vmware.com%7C27a7ee62d7734fc2000b08d7e46cb8be%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637229027923593475&sdata=rKm%2BRZE%2FPWkvNTinHgQ5qgA9EDcD4lyIZSZMsHf66d0%3D&reserved=0 > (See build_firmware.sh), the reasoning > being that if someone encounters an issue with RELEASE and we ask them > to troubleshoot with the DEBUG artifact, we want to eliminate potential > troublemakers when they try that. > >> It is a non-standard hack that ARM contributed in the past, and is not >> covered by the EFI of Shell specifications. If RPi4 is intended to be a >> showcase for UEFI on ARM done right, we should not enable this at all. > > Here I have to point out that RPi4 becoming a showcase because we intend > to is not what we are pursuing (because if it was a matter of "willing" > a showcase into existence, we would have picked a platform with a lot > less quirks, more comprehensive documentation, and so on). > > Instead, we estimate that due to its price point and widespread > availability, it *is* going to become a de facto showcase, whether > everybody likes it or not. And that is the reason we want to treat is as > a showcase where possible. > > Regards, > > /Pete > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#57575): https://edk2.groups.io/g/devel/message/57575 Mute This Topic: https://groups.io/mt/73127191/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
So if I understood correctly: * If a random person off the street builds edk2 - they don't get TFTP command out of the box * Our builds retain TFTP command Correct? ________________________________ From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Pete Batard via groups.io <pete=akeo.ie@groups.io> Sent: Sunday, April 19, 2020 3:06 PM To: devel@edk2.groups.io <devel@edk2.groups.io>; Andrei Warkentin <awarkentin@vmware.com>; Ard Biesheuvel <ard.biesheuvel@arm.com>; Samer El-Haj-Mahmoud <samer@elhajmahmoud.com> Cc: Leif Lindholm <leif@nuviainc.com> Subject: Re: [edk2-devel] [edk2-platform][PATCH v1 0/4] Platform/RaspberryPi : Enable TFTP shell command Andrei, In case this is your concern, please note that we are not removing TFTP support at all, which is enabled for the RELEASE builds we produce and will remain so (and which anyone can enable with the macro if they wish). All that will be changed by the updated proposal is that the current DEBUG ASSERT will be fixed and TFTP support will remain optional, like it is today. So, in this case, I don't think your concern is warranted, because we're not actually taking any step to deprive anyone of any functionality they might wish for, and, even with the revised patch, TFTP will remain enabled in our RELEASE binaries, exactly as it has been before. Regards, /Pete On 2020.04.19 20:56, Andrei Warkentin wrote: > Hi folks, > > If we have to choose abstract goodness over functionality, why wouldn't > we choose functionality? Functionality that's part of Tiano? The real > world doesn't care about the TFTP command being an "unsupported hack" or > not. So there's Tiano-specific code here. Big deal? To rephrase > differently, why would either Pi 4 developers or Pi 4 UEFI users pay the > cost of Tiano carrying code that somehow isn't "legit enough" to be enabled? > > I mean here we are again, where what goes into the code is being > dictated by some abstract ideology instead of technical reasons? > > A > ------------------------------------------------------------------------ > *From:* Pete Batard <pete@akeo.ie> > *Sent:* Sunday, April 19, 2020 9:19 AM > *To:* Ard Biesheuvel <ard.biesheuvel@arm.com>; Samer El-Haj-Mahmoud > <samer@elhajmahmoud.com>; devel@edk2.groups.io <devel@edk2.groups.io> > *Cc:* Leif Lindholm <leif@nuviainc.com>; Andrei Warkentin > <awarkentin@vmware.com> > *Subject:* Re: [edk2-platform][PATCH v1 0/4] Platform/RaspberryPi : > Enable TFTP shell command > On 2020.04.19 14:33, Ard Biesheuvel wrote: >> On 4/19/20 3:04 PM, Samer El-Haj-Mahmoud wrote: >>> Fix an ASSERT with the TFTP dynamic Shell command on the >>> RPi3 and RPi4 when running DEBUG builds. Also, enable the >>> command by default for all builds. >>> >> >> Fixing the ASSERT is fine but I am reluctant to enable this by default. > > I'm going to second this. > > To answer a question Samer was asking elsewhere, this is actually part > of the reason why TFTP is not enabled in the DEBUG builds we produce at > https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fpftf%2FRPi4&data=02%7C01%7Cawarkentin%40vmware.com%7C56cfed340a494707ab0d08d7e49d2d64%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637229236035259614&sdata=Ed8iabD5ARgBBYtsWgGw2Webu7dAW5Q9ju3qyqyVzX4%3D&reserved=0 > (See build_firmware.sh), the reasoning > being that if someone encounters an issue with RELEASE and we ask them > to troubleshoot with the DEBUG artifact, we want to eliminate potential > troublemakers when they try that. > >> It is a non-standard hack that ARM contributed in the past, and is not >> covered by the EFI of Shell specifications. If RPi4 is intended to be a >> showcase for UEFI on ARM done right, we should not enable this at all. > > Here I have to point out that RPi4 becoming a showcase because we intend > to is not what we are pursuing (because if it was a matter of "willing" > a showcase into existence, we would have picked a platform with a lot > less quirks, more comprehensive documentation, and so on). > > Instead, we estimate that due to its price point and widespread > availability, it *is* going to become a de facto showcase, whether > everybody likes it or not. And that is the reason we want to treat is as > a showcase where possible. > > Regards, > > /Pete > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#57576): https://edk2.groups.io/g/devel/message/57576 Mute This Topic: https://groups.io/mt/73127191/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On 2020.04.19 21:21, awarkentin@vmware.com wrote: > So if I understood correctly: > > * If a random person off the street builds edk2 - they don't get TFTP > command out of the box Yup. For the reasons that Ard pointed out (current TFTP being a non-standard hack that should be replaced by something more suitable... eventually). > * Our builds retain TFTP command Yup. > > Correct? > ------------------------------------------------------------------------ > *From:* devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Pete > Batard via groups.io <pete=akeo.ie@groups.io> > *Sent:* Sunday, April 19, 2020 3:06 PM > *To:* devel@edk2.groups.io <devel@edk2.groups.io>; Andrei Warkentin > <awarkentin@vmware.com>; Ard Biesheuvel <ard.biesheuvel@arm.com>; Samer > El-Haj-Mahmoud <samer@elhajmahmoud.com> > *Cc:* Leif Lindholm <leif@nuviainc.com> > *Subject:* Re: [edk2-devel] [edk2-platform][PATCH v1 0/4] > Platform/RaspberryPi : Enable TFTP shell command > Andrei, > > In case this is your concern, please note that we are not removing TFTP > support at all, which is enabled for the RELEASE builds we produce and > will remain so (and which anyone can enable with the macro if they wish). > > All that will be changed by the updated proposal is that the current > DEBUG ASSERT will be fixed and TFTP support will remain optional, like > it is today. > > So, in this case, I don't think your concern is warranted, because we're > not actually taking any step to deprive anyone of any functionality they > might wish for, and, even with the revised patch, TFTP will remain > enabled in our RELEASE binaries, exactly as it has been before. > > Regards, > > /Pete > > On 2020.04.19 20:56, Andrei Warkentin wrote: >> Hi folks, >> >> If we have to choose abstract goodness over functionality, why wouldn't >> we choose functionality? Functionality that's part of Tiano? The real >> world doesn't care about the TFTP command being an "unsupported hack" or >> not. So there's Tiano-specific code here. Big deal? To rephrase >> differently, why would either Pi 4 developers or Pi 4 UEFI users pay the >> cost of Tiano carrying code that somehow isn't "legit enough" to be enabled? >> >> I mean here we are again, where what goes into the code is being >> dictated by some abstract ideology instead of technical reasons? >> >> A >> ------------------------------------------------------------------------ >> *From:* Pete Batard <pete@akeo.ie> >> *Sent:* Sunday, April 19, 2020 9:19 AM >> *To:* Ard Biesheuvel <ard.biesheuvel@arm.com>; Samer El-Haj-Mahmoud >> <samer@elhajmahmoud.com>; devel@edk2.groups.io <devel@edk2.groups.io> >> *Cc:* Leif Lindholm <leif@nuviainc.com>; Andrei Warkentin >> <awarkentin@vmware.com> >> *Subject:* Re: [edk2-platform][PATCH v1 0/4] Platform/RaspberryPi : >> Enable TFTP shell command >> On 2020.04.19 14:33, Ard Biesheuvel wrote: >>> On 4/19/20 3:04 PM, Samer El-Haj-Mahmoud wrote: >>>> Fix an ASSERT with the TFTP dynamic Shell command on the >>>> RPi3 and RPi4 when running DEBUG builds. Also, enable the >>>> command by default for all builds. >>>> >>> >>> Fixing the ASSERT is fine but I am reluctant to enable this by default. >> >> I'm going to second this. >> >> To answer a question Samer was asking elsewhere, this is actually part >> of the reason why TFTP is not enabled in the DEBUG builds we produce at >> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fpftf%2FRPi4&data=02%7C01%7Cawarkentin%40vmware.com%7C56cfed340a494707ab0d08d7e49d2d64%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637229236035259614&sdata=Ed8iabD5ARgBBYtsWgGw2Webu7dAW5Q9ju3qyqyVzX4%3D&reserved=0 > >> (See build_firmware.sh), the reasoning >> being that if someone encounters an issue with RELEASE and we ask them >> to troubleshoot with the DEBUG artifact, we want to eliminate potential >> troublemakers when they try that. >> >>> It is a non-standard hack that ARM contributed in the past, and is not >>> covered by the EFI of Shell specifications. If RPi4 is intended to be a >>> showcase for UEFI on ARM done right, we should not enable this at all. >> >> Here I have to point out that RPi4 becoming a showcase because we intend >> to is not what we are pursuing (because if it was a matter of "willing" >> a showcase into existence, we would have picked a platform with a lot >> less quirks, more comprehensive documentation, and so on). >> >> Instead, we estimate that due to its price point and widespread >> availability, it *is* going to become a de facto showcase, whether >> everybody likes it or not. And that is the reason we want to treat is as >> a showcase where possible. >> >> Regards, >> >> /Pete >> > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#57577): https://edk2.groups.io/g/devel/message/57577 Mute This Topic: https://groups.io/mt/73127191/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
It's part of Tiano, no? We didn't develop it. Yet I see it being used in many Tiano-derived UEFI implementations in the Arm world. I don't see a contract anywhere that all Tiano implementations ought to avoid components that don't fit the UEFI/PI/Shell specs. Can someone point me to such a contract? We're entering the "victimless crime" territory here, and also violating the principle of least surprise. I do agree that DEBUG profile may choose a subset of configuration options, for reasons such as sticking to a smaller configuration (for size, complexity, etc). A ________________________________ From: Pete Batard <pete@akeo.ie> Sent: Sunday, April 19, 2020 3:24 PM To: Andrei Warkentin <awarkentin@vmware.com>; devel@edk2.groups.io <devel@edk2.groups.io>; Ard Biesheuvel <ard.biesheuvel@arm.com>; Samer El-Haj-Mahmoud <samer@elhajmahmoud.com> Cc: Leif Lindholm <leif@nuviainc.com> Subject: Re: [edk2-devel] [edk2-platform][PATCH v1 0/4] Platform/RaspberryPi : Enable TFTP shell command On 2020.04.19 21:21, awarkentin@vmware.com wrote: > So if I understood correctly: > > * If a random person off the street builds edk2 - they don't get TFTP > command out of the box Yup. For the reasons that Ard pointed out (current TFTP being a non-standard hack that should be replaced by something more suitable... eventually). > * Our builds retain TFTP command Yup. > > Correct? > ------------------------------------------------------------------------ > *From:* devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Pete > Batard via groups.io <pete=akeo.ie@groups.io> > *Sent:* Sunday, April 19, 2020 3:06 PM > *To:* devel@edk2.groups.io <devel@edk2.groups.io>; Andrei Warkentin > <awarkentin@vmware.com>; Ard Biesheuvel <ard.biesheuvel@arm.com>; Samer > El-Haj-Mahmoud <samer@elhajmahmoud.com> > *Cc:* Leif Lindholm <leif@nuviainc.com> > *Subject:* Re: [edk2-devel] [edk2-platform][PATCH v1 0/4] > Platform/RaspberryPi : Enable TFTP shell command > Andrei, > > In case this is your concern, please note that we are not removing TFTP > support at all, which is enabled for the RELEASE builds we produce and > will remain so (and which anyone can enable with the macro if they wish). > > All that will be changed by the updated proposal is that the current > DEBUG ASSERT will be fixed and TFTP support will remain optional, like > it is today. > > So, in this case, I don't think your concern is warranted, because we're > not actually taking any step to deprive anyone of any functionality they > might wish for, and, even with the revised patch, TFTP will remain > enabled in our RELEASE binaries, exactly as it has been before. > > Regards, > > /Pete > > On 2020.04.19 20:56, Andrei Warkentin wrote: >> Hi folks, >> >> If we have to choose abstract goodness over functionality, why wouldn't >> we choose functionality? Functionality that's part of Tiano? The real >> world doesn't care about the TFTP command being an "unsupported hack" or >> not. So there's Tiano-specific code here. Big deal? To rephrase >> differently, why would either Pi 4 developers or Pi 4 UEFI users pay the >> cost of Tiano carrying code that somehow isn't "legit enough" to be enabled? >> >> I mean here we are again, where what goes into the code is being >> dictated by some abstract ideology instead of technical reasons? >> >> A >> ------------------------------------------------------------------------ >> *From:* Pete Batard <pete@akeo.ie> >> *Sent:* Sunday, April 19, 2020 9:19 AM >> *To:* Ard Biesheuvel <ard.biesheuvel@arm.com>; Samer El-Haj-Mahmoud >> <samer@elhajmahmoud.com>; devel@edk2.groups.io <devel@edk2.groups.io> >> *Cc:* Leif Lindholm <leif@nuviainc.com>; Andrei Warkentin >> <awarkentin@vmware.com> >> *Subject:* Re: [edk2-platform][PATCH v1 0/4] Platform/RaspberryPi : >> Enable TFTP shell command >> On 2020.04.19 14:33, Ard Biesheuvel wrote: >>> On 4/19/20 3:04 PM, Samer El-Haj-Mahmoud wrote: >>>> Fix an ASSERT with the TFTP dynamic Shell command on the >>>> RPi3 and RPi4 when running DEBUG builds. Also, enable the >>>> command by default for all builds. >>>> >>> >>> Fixing the ASSERT is fine but I am reluctant to enable this by default. >> >> I'm going to second this. >> >> To answer a question Samer was asking elsewhere, this is actually part >> of the reason why TFTP is not enabled in the DEBUG builds we produce at >> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fpftf%2FRPi4&data=02%7C01%7Cawarkentin%40vmware.com%7Cff25433f108e490d28fc08d7e49fa694%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637229246665197560&sdata=rgUNgoQgPZGgfcQaQfzE6hn3WNj1Y5sgv6Zr9pbCJGg%3D&reserved=0 > >> (See build_firmware.sh), the reasoning >> being that if someone encounters an issue with RELEASE and we ask them >> to troubleshoot with the DEBUG artifact, we want to eliminate potential >> troublemakers when they try that. >> >>> It is a non-standard hack that ARM contributed in the past, and is not >>> covered by the EFI of Shell specifications. If RPi4 is intended to be a >>> showcase for UEFI on ARM done right, we should not enable this at all. >> >> Here I have to point out that RPi4 becoming a showcase because we intend >> to is not what we are pursuing (because if it was a matter of "willing" >> a showcase into existence, we would have picked a platform with a lot >> less quirks, more comprehensive documentation, and so on). >> >> Instead, we estimate that due to its price point and widespread >> availability, it *is* going to become a de facto showcase, whether >> everybody likes it or not. And that is the reason we want to treat is as >> a showcase where possible. >> >> Regards, >> >> /Pete >> > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#57578): https://edk2.groups.io/g/devel/message/57578 Mute This Topic: https://groups.io/mt/73127191/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Hi Andrei,
On Sun, Apr 19, 2020 at 19:56:32 +0000, awarkentin@vmware.com wrote:
> If we have to choose abstract goodness over functionality, why
> wouldn't we choose functionality? Functionality that's part of
> Tiano? The real world doesn't care about the TFTP command being an
> "unsupported hack" or not. So there's Tiano-specific code here. Big
> deal? To rephrase differently, why would either Pi 4 developers or
> Pi 4 UEFI users pay the cost of Tiano carrying code that somehow
> isn't "legit enough" to be enabled?
I agree there is confusion caused by this weird second implementation
of TFTP in Tianocore. It is yet another piece of unfortunate legacy
caused by ARM's initial focus on trying to make EDK2 look and work
like u-boot instead of understanding how to use it as a generic UEFI
implementation.
For that reason, it should have gone the same way as the ArmBds and
the built-in linux loader, but people kept arguing it was really
useful for debugging - so we let it be, and permitted platforms to
include it as long as it was not included by default.
But since its main contribution to TianoCore seems to be causing
arguments on the mailing list, perhaps we should finally bite the
bullet and nuke it.
The idea (which was probably mine) that "only permit platforms to
include it if it is disabled by default and people will get the hint"
has demonstrably failed.
/
Leif
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#57613): https://edk2.groups.io/g/devel/message/57613
Mute This Topic: https://groups.io/mt/73127191/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
I have no objection to dropping the "enable TFTP by default". I already stated that to Ard in my original reply. That is not really a big issue (or an issue at all) since we have the -D build option to enable the command in some builds, and the net is no functionality change for end-users. In general, I am in favor of having Shell commands that are wrappers around useful UEFI protocols/functionality where it makes sense (e.g. would be nice to have a CLI HII forms processor/browser..). I thought the TFTP command offered that (a wrapper around the MTFTP4 protocol), which helps in some Shell remote scripting (although honestly, not very useful on its own). I understand that there is hesitation in including non-standard commands in the Shell by default. I did not consider TFTP as a replacement for Capsule Updates, but I can see how it can be abused to be so. I agree that usage should not be encouraged. I still have not seen a review-by or ack for the ASSERT fix. Can we get patch 1 and 2 in the series reviewed and pushed please, and ignore patches 3/4? Thanks, --Samer > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Leif > Lindholm via groups.io > Sent: Monday, April 20, 2020 7:23 AM > To: Andrei Warkentin (awarkentin@vmware.com) > <awarkentin@vmware.com>; Ard Biesheuvel <Ard.Biesheuvel@arm.com> > Cc: Pete Batard <pete@akeo.ie>; Samer El-Haj-Mahmoud > <samer@elhajmahmoud.com>; devel@edk2.groups.io > Subject: Re: [edk2-devel] [edk2-platform][PATCH v1 0/4] Platform/RaspberryPi > : Enable TFTP shell command > > Hi Andrei, > > On Sun, Apr 19, 2020 at 19:56:32 +0000, awarkentin@vmware.com wrote: > > If we have to choose abstract goodness over functionality, why > > wouldn't we choose functionality? Functionality that's part of Tiano? > > The real world doesn't care about the TFTP command being an > > "unsupported hack" or not. So there's Tiano-specific code here. Big > > deal? To rephrase differently, why would either Pi 4 developers or Pi > > 4 UEFI users pay the cost of Tiano carrying code that somehow isn't > > "legit enough" to be enabled? > > I agree there is confusion caused by this weird second implementation of TFTP > in Tianocore. It is yet another piece of unfortunate legacy caused by ARM's > initial focus on trying to make EDK2 look and work like u-boot instead of > understanding how to use it as a generic UEFI implementation. > > For that reason, it should have gone the same way as the ArmBds and the built- > in linux loader, but people kept arguing it was really useful for debugging - so > we let it be, and permitted platforms to include it as long as it was not included > by default. > > But since its main contribution to TianoCore seems to be causing arguments on > the mailing list, perhaps we should finally bite the bullet and nuke it. > > The idea (which was probably mine) that "only permit platforms to include it if > it is disabled by default and people will get the hint" > has demonstrably failed. > > / > Leif > > IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#57630): https://edk2.groups.io/g/devel/message/57630 Mute This Topic: https://groups.io/mt/73127191/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On 4/19/20 3:04 PM, Samer El-Haj-Mahmoud wrote: > Fix an ASSERT with the TFTP dynamic Shell command on the > RPi3 and RPi4 when running DEBUG builds. Also, enable the > command by default for all builds. > > Cc: Leif Lindholm <leif@nuviainc.com> > Cc: Ard Biesheuvel <ard.biesheuvel@arm.com> > Cc: Pete Batard <pete@akeo.ie> > Cc: Andrei Warkentin <awarkentin@vmware.com> > > Samer El-Haj-Mahmoud (4): > Platform/RaspberryPi/RPi3: Fix TFTP dynamic command initialization > Platform/RaspberryPi/RPi4: Fix TFTP dynamic command initialization Patches #1 and #2 Reviewed-by: Ard Biesheuvel <ard.biesheuvel@arm.com> Pushed as 944af47a8c83..f09ea1a6227f -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#57634): https://edk2.groups.io/g/devel/message/57634 Mute This Topic: https://groups.io/mt/73127191/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Thanks Ard! > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ard > Biesheuvel via groups.io > Sent: Monday, April 20, 2020 9:25 AM > To: Samer El-Haj-Mahmoud <samer@elhajmahmoud.com>; > devel@edk2.groups.io > Cc: Leif Lindholm <leif@nuviainc.com>; Pete Batard <pete@akeo.ie>; Andrei > Warkentin (awarkentin@vmware.com) <awarkentin@vmware.com> > Subject: Re: [edk2-devel] [edk2-platform][PATCH v1 0/4] Platform/RaspberryPi > : Enable TFTP shell command > > On 4/19/20 3:04 PM, Samer El-Haj-Mahmoud wrote: > > Fix an ASSERT with the TFTP dynamic Shell command on the > > RPi3 and RPi4 when running DEBUG builds. Also, enable the command by > > default for all builds. > > > > Cc: Leif Lindholm <leif@nuviainc.com> > > Cc: Ard Biesheuvel <ard.biesheuvel@arm.com> > > Cc: Pete Batard <pete@akeo.ie> > > Cc: Andrei Warkentin <awarkentin@vmware.com> > > > > Samer El-Haj-Mahmoud (4): > > Platform/RaspberryPi/RPi3: Fix TFTP dynamic command initialization > > Platform/RaspberryPi/RPi4: Fix TFTP dynamic command initialization > > Patches #1 and #2 > > Reviewed-by: Ard Biesheuvel <ard.biesheuvel@arm.com> > > Pushed as 944af47a8c83..f09ea1a6227f > > > IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#57635): https://edk2.groups.io/g/devel/message/57635 Mute This Topic: https://groups.io/mt/73127191/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.