Hi all,
I've spent some time over the past couple of days getting update-grub working on the hikey. It works for me, but I need some review. The solution consists of three parts:
1) a new config file in /etc/default/kernel that can contain KERNEL_DEVICETREE="<board-name>" 2) a new script in /etc/kernel/postinst.d/devicetree that copies the requested device tree file into /boot when KERNEL_DEVICETREE is set 3) a modifcation to /etc/grub.d/10_linux that will add the devicetree command when KERNEL_DEVICETREE is set.
I've also be partially successful using the grub-install tool. It will install the grubaa64.efi application correctly in (ESP)/EFI/debian/grubaa64.efi, but barfs when trying to update the EFI variables which is unsurprising. However, the system boots fine when I copy the installed grubaa64.efi into (ESP)/EFI/BOOT/grubaa64.efi
With these changes I can install a kernel package, and grub gets updated correctly with the new kernel information.
On Mon, Feb 08, 2016 at 03:35:36PM +0000, Grant Likely wrote:
Hi all,
Hi Grant,
I've spent some time over the past couple of days getting update-grub working on the hikey. It works for me, but I need some review. The solution consists of three parts:
- a new config file in /etc/default/kernel that can contain
KERNEL_DEVICETREE="<board-name>" 2) a new script in /etc/kernel/postinst.d/devicetree that copies the requested device tree file into /boot when KERNEL_DEVICETREE is set 3) a modifcation to /etc/grub.d/10_linux that will add the devicetree command when KERNEL_DEVICETREE is set.
I've also be partially successful using the grub-install tool. It will install the grubaa64.efi application correctly in (ESP)/EFI/debian/grubaa64.efi, but barfs when trying to update the EFI variables which is unsurprising. However, the system boots fine when I copy the installed grubaa64.efi into (ESP)/EFI/BOOT/grubaa64.efi
I believe this is because the HiKey EDK2 port has hard-coded assumptions about particular file paths (e.g. /EFI/BOOT/grubaa64.efi) beyond that specified for removable media (e.g. /EFI/boot/bootaa64.efi).
That seems like a bug (and is certainly not in the spirit of EFI).
With these changes I can install a kernel package, and grub gets updated correctly with the new kernel information.
I'm rather worried by this, and very much not keen on having distro-packaged GRUB have to know anything about what should be a system-provided DTB.
While this does seem to make it easier for users in the short term, it disincentivizes doing the right thing, and pushes a lot of pain back to the upstream kernel and distros (in having to maintain those DTBs, packages and updates thereof, etc). I don't think that's generally a good thing.
DTBs shouldn't get broken arbitrarily, so there's no strict need to run the latest and greatest DTB. What prevents the HiKey firmware from providing a minimal but correct DTB for now? That can be updated as time goes on (e.g. with capsule updates).
That should work fine for end users. Developers will be messing with custom bleeding-edge DTBs regardless of distro packaging.
I think this causes more long term pain than the short term pain it solves, and I think that it hides the pain from those who should be feeling it.
Thanks, Mark.
Description: Add support to automatically specify a devicetree file . Add support for automatically adding the devicetree line to a linux boot stanza if the system configuration asks for it. Uses a new file in /etc/default/kernel to set a variable for the system .dtb file name. . Author: Grant Likely grant.likely@linaro.org
The information above should follow the Patch Tagging Guidelines, please checkout http://dep.debian.net/deps/dep3/ to learn about the format. Here are templates for supplementary fields that you might want to add:
Origin: <vendor|upstream|other>, <url of original patch> Bug: <url in upstream bugtracker> Bug-Debian: https://bugs.debian.org/<bugnumber> Bug-Ubuntu: https://launchpad.net/bugs/<bugnumber> Forwarded: <no|not-needed|url proving that it has been forwarded> Reviewed-By: <name and email of someone who approved the patch> Last-Update: <YYYY-MM-DD>
Index: grub2-2.02~beta2/util/grub.d/10_linux.in
--- grub2-2.02~beta2.orig/util/grub.d/10_linux.in +++ grub2-2.02~beta2/util/grub.d/10_linux.in @@ -27,6 +27,9 @@ gfxpayload_dynamic="@GFXPAYLOAD_DYNAMIC@ vt_handoff="@VT_HANDOFF@" . "@datadir@/@PACKAGE@/grub-mkconfig_lib" +if [ -e /etc/default/kernel ] ; then
- . /etc/default/kernel
+fi export TEXTDOMAIN=@PACKAGE@ export TEXTDOMAINDIR="@localedir@" @@ -193,6 +196,17 @@ EOF initrd ${rel_dirname}/${initrd} EOF fi
- if test -n "${devicetree}" ; then
- if [ x"$quiet_boot" = x0 ] || [ x"$type" != xsimple ]; then
message="$(gettext_printf "Loading devicetree ...")"sed "s/^/$submenu_indentation/" << EOF- echo '$(echo "$message" | grub_quote)'
+EOF
- fi
- sed "s/^/$submenu_indentation/" << EOF
- devicetree ${rel_dirname}/${devicetree}
+EOF
- fi sed "s/^/$submenu_indentation/" << EOF
} EOF @@ -327,6 +341,16 @@ while [ "x$list" != "x" ] ; do linux_root_device_thisversion=${GRUB_DEVICE} fi
- devicetree=
- for i in "${KERNEL_DEVICETREE}-${version}.dtb" \
"${KERNEL_DEVICETREE}-${alt_version}.dtb" ; do- if test -e "${dirname}/${i}" ; then
devicetree="${i}"gettext_printf "Found devicetree image: %s\n" "${dirname}/${devicetree}" >&2break- fi
- done
- if [ "x$is_top_level" = xtrue ] && [ "x${GRUB_DISABLE_SUBMENU}" != xy ]; then linux_entry "${OS}" "${version}" simple \ "${GRUB_CMDLINE_LINUX} ${GRUB_CMDLINE_LINUX_DEFAULT}"
Dev mailing list Dev@lists.96boards.org https://lists.96boards.org/mailman/listinfo/dev
On Mon, Feb 08, 2016 at 03:35:36PM +0000, Grant Likely wrote:
Hi all,
I've spent some time over the past couple of days getting update-grub working on the hikey. It works for me, but I need some review. The solution consists of three parts:
- a new config file in /etc/default/kernel that can contain
KERNEL_DEVICETREE="<board-name>" 2) a new script in /etc/kernel/postinst.d/devicetree that copies the requested device tree file into /boot when KERNEL_DEVICETREE is set 3) a modifcation to /etc/grub.d/10_linux that will add the devicetree command when KERNEL_DEVICETREE is set.
Hmmmm. Other machines aren't needing to know the specific devicetree in this way.
I've also be partially successful using the grub-install tool. It will install the grubaa64.efi application correctly in (ESP)/EFI/debian/grubaa64.efi, but barfs when trying to update the EFI variables which is unsurprising. However, the system boots fine when I copy the installed grubaa64.efi into (ESP)/EFI/BOOT/grubaa64.efi
Oh, ugh. What's the problem with the EFI variables? From a Debian perspective, you could (almost) get away with using the workaround I developed for broken firmware that doesn't do EFI properly. See http://bugs.debian.org/746662 for the full story if you're interested. There's support in Debian to track this automatically, and it's preseedable too if you need that.
However (as Mark says), if it's looking for (ESP)/EFI/BOOT/grubaa64.efi then it's wrong. The removable media path should be (ESP)/EFI/BOOT/bootaa64.efi, or have you mis-typed that in your mail here? (I hope so!)
Cheers,
On Mon, Feb 8, 2016 at 2:37 PM, Steve McIntyre steve.mcintyre@linaro.org wrote:
On Mon, Feb 08, 2016 at 03:35:36PM +0000, Grant Likely wrote:
Hi all,
I've spent some time over the past couple of days getting update-grub working on the hikey. It works for me, but I need some review. The solution consists of three parts:
- a new config file in /etc/default/kernel that can contain
KERNEL_DEVICETREE="<board-name>" 2) a new script in /etc/kernel/postinst.d/devicetree that copies the requested device tree file into /boot when KERNEL_DEVICETREE is set 3) a modifcation to /etc/grub.d/10_linux that will add the devicetree command when KERNEL_DEVICETREE is set.
Hmmmm. Other machines aren't needing to know the specific devicetree in this way.
That is probably because the firmware is providing a valid device tree by default.
I've also be partially successful using the grub-install tool. It will install the grubaa64.efi application correctly in (ESP)/EFI/debian/grubaa64.efi, but barfs when trying to update the EFI variables which is unsurprising. However, the system boots fine when I copy the installed grubaa64.efi into (ESP)/EFI/BOOT/grubaa64.efi
Oh, ugh. What's the problem with the EFI variables? From a Debian perspective, you could (almost) get away with using the workaround I developed for broken firmware that doesn't do EFI properly. See http://bugs.debian.org/746662 for the full story if you're interested. There's support in Debian to track this automatically, and it's preseedable too if you need that.
Isn't the variables here the runtime support (for efibootmng)? If so, that is not supported by hikey.
However (as Mark says), if it's looking for (ESP)/EFI/BOOT/grubaa64.efi then it's wrong. The removable media path should be (ESP)/EFI/BOOT/bootaa64.efi, or have you mis-typed that in your mail here? (I hope so!)
This is probably why (which is known to be broken):
HisiPkg/HiKeyPkg/Drivers/HiKeyDxe/InstallBootMenu.c [HIKEY_BOOT_ENTRY_BOOT_EMMC] = { L"VenHw(B549F005-4BD4-4020-A0CB-06F42BDA68C3)/HD(6,GPT,5C0F213C-17E1-4149-88C8-8B50FB4EC70E,0x7000,0x20000)/\EFI\BOOT\GRUBAA64.EFI", NULL, L"boot from eMMC", LOAD_OPTION_CATEGORY_APP },
I'm hoping this to be fixed with the edk2/openplatformpkg rework and upstreaming effort that was started a few weeks ago.
Cheers,
On Mon, Feb 8, 2016 at 2:06 PM, Mark Rutland mark.rutland@arm.com wrote:
On Mon, Feb 08, 2016 at 03:35:36PM +0000, Grant Likely wrote:
Hi all,
Hi Grant,
I've spent some time over the past couple of days getting update-grub working on the hikey. It works for me, but I need some review. The solution consists of three parts:
- a new config file in /etc/default/kernel that can contain
KERNEL_DEVICETREE="<board-name>" 2) a new script in /etc/kernel/postinst.d/devicetree that copies the requested device tree file into /boot when KERNEL_DEVICETREE is set 3) a modifcation to /etc/grub.d/10_linux that will add the devicetree command when KERNEL_DEVICETREE is set.
I've also be partially successful using the grub-install tool. It will install the grubaa64.efi application correctly in (ESP)/EFI/debian/grubaa64.efi, but barfs when trying to update the EFI variables which is unsurprising. However, the system boots fine when I copy the installed grubaa64.efi into (ESP)/EFI/BOOT/grubaa64.efi
I believe this is because the HiKey EDK2 port has hard-coded assumptions about particular file paths (e.g. /EFI/BOOT/grubaa64.efi) beyond that specified for removable media (e.g. /EFI/boot/bootaa64.efi).
That seems like a bug (and is certainly not in the spirit of EFI).
With these changes I can install a kernel package, and grub gets updated correctly with the new kernel information.
I'm rather worried by this, and very much not keen on having distro-packaged GRUB have to know anything about what should be a system-provided DTB.
While this does seem to make it easier for users in the short term, it disincentivizes doing the right thing, and pushes a lot of pain back to the upstream kernel and distros (in having to maintain those DTBs, packages and updates thereof, etc). I don't think that's generally a good thing.
DTBs shouldn't get broken arbitrarily, so there's no strict need to run the latest and greatest DTB. What prevents the HiKey firmware from providing a minimal but correct DTB for now? That can be updated as time goes on (e.g. with capsule updates).
That should work fine for end users. Developers will be messing with custom bleeding-edge DTBs regardless of distro packaging.
I think this causes more long term pain than the short term pain it solves, and I think that it hides the pain from those who should be feeling it.
I think this is quite useful for development (since there is quite a bit of upstreaming going now, finally), but I also agree that it might be better to just fix/improve our firmware.
If we get the firmware right (by providing the DTB), it will be easier for everyone using hikey with any distro.
Cheers,
On Mon, Feb 08, 2016 at 02:49:59PM -0200, Ricardo Salveti wrote:
On Mon, Feb 8, 2016 at 2:37 PM, Steve McIntyre steve.mcintyre@linaro.org wrote:
On Mon, Feb 08, 2016 at 03:35:36PM +0000, Grant Likely wrote:
Hi all,
I've spent some time over the past couple of days getting update-grub working on the hikey. It works for me, but I need some review. The solution consists of three parts:
- a new config file in /etc/default/kernel that can contain
KERNEL_DEVICETREE="<board-name>" 2) a new script in /etc/kernel/postinst.d/devicetree that copies the requested device tree file into /boot when KERNEL_DEVICETREE is set 3) a modifcation to /etc/grub.d/10_linux that will add the devicetree command when KERNEL_DEVICETREE is set.
Hmmmm. Other machines aren't needing to know the specific devicetree in this way.
That is probably because the firmware is providing a valid device tree by default.
I've also be partially successful using the grub-install tool. It will install the grubaa64.efi application correctly in (ESP)/EFI/debian/grubaa64.efi, but barfs when trying to update the EFI variables which is unsurprising. However, the system boots fine when I copy the installed grubaa64.efi into (ESP)/EFI/BOOT/grubaa64.efi
Oh, ugh. What's the problem with the EFI variables? From a Debian perspective, you could (almost) get away with using the workaround I developed for broken firmware that doesn't do EFI properly. See http://bugs.debian.org/746662 for the full story if you're interested. There's support in Debian to track this automatically, and it's preseedable too if you need that.
Isn't the variables here the runtime support (for efibootmng)?
Yes. The boot variables are like any other variables, and are manipulated via runtime services (e.g. SetVariable).
If so, that is not supported by hikey.
Is there any route to having them supported? I don't know enough about HiKey to know whether it's not possible, very difficult, or simply not worked on.
They are required per the EFI spec, and it's an unfortunate divergence given they're key to the usual boot flow.
Thanks, Mark.
On Mon, Feb 8, 2016 at 3:17 PM, Mark Rutland mark.rutland@arm.com wrote:
On Mon, Feb 08, 2016 at 02:49:59PM -0200, Ricardo Salveti wrote:
On Mon, Feb 8, 2016 at 2:37 PM, Steve McIntyre steve.mcintyre@linaro.org wrote:
On Mon, Feb 08, 2016 at 03:35:36PM +0000, Grant Likely wrote:
Hi all,
I've spent some time over the past couple of days getting update-grub working on the hikey. It works for me, but I need some review. The solution consists of three parts:
- a new config file in /etc/default/kernel that can contain
KERNEL_DEVICETREE="<board-name>" 2) a new script in /etc/kernel/postinst.d/devicetree that copies the requested device tree file into /boot when KERNEL_DEVICETREE is set 3) a modifcation to /etc/grub.d/10_linux that will add the devicetree command when KERNEL_DEVICETREE is set.
Hmmmm. Other machines aren't needing to know the specific devicetree in this way.
That is probably because the firmware is providing a valid device tree by default.
I've also be partially successful using the grub-install tool. It will install the grubaa64.efi application correctly in (ESP)/EFI/debian/grubaa64.efi, but barfs when trying to update the EFI variables which is unsurprising. However, the system boots fine when I copy the installed grubaa64.efi into (ESP)/EFI/BOOT/grubaa64.efi
Oh, ugh. What's the problem with the EFI variables? From a Debian perspective, you could (almost) get away with using the workaround I developed for broken firmware that doesn't do EFI properly. See http://bugs.debian.org/746662 for the full story if you're interested. There's support in Debian to track this automatically, and it's preseedable too if you need that.
Isn't the variables here the runtime support (for efibootmng)?
Yes. The boot variables are like any other variables, and are manipulated via runtime services (e.g. SetVariable).
If so, that is not supported by hikey.
Is there any route to having them supported? I don't know enough about HiKey to know whether it's not possible, very difficult, or simply not worked on.
They are required per the EFI spec, and it's an unfortunate divergence given they're key to the usual boot flow.
Haojian, since you are restructuring the code as we speak, would it be even possible to support runtime here?
Since you are already storing meta-data in a specific disk partition, wouldn't it be possible to reuse the same logic to make runtime supported?
Cheers,
On Mon, Feb 08, 2016 at 02:53:19PM -0200, Ricardo Salveti wrote:
On Mon, Feb 8, 2016 at 2:06 PM, Mark Rutland mark.rutland@arm.com wrote:
DTBs shouldn't get broken arbitrarily, so there's no strict need to run the latest and greatest DTB. What prevents the HiKey firmware from providing a minimal but correct DTB for now? That can be updated as time goes on (e.g. with capsule updates).
That should work fine for end users. Developers will be messing with custom bleeding-edge DTBs regardless of distro packaging.
I think this causes more long term pain than the short term pain it solves, and I think that it hides the pain from those who should be feeling it.
I think this is quite useful for development (since there is quite a bit of upstreaming going now, finally), but I also agree that it might be better to just fix/improve our firmware.
If we get the firmware right (by providing the DTB), it will be easier for everyone using hikey with any distro.
Definitely.
Cheers,
On Mon, Feb 08, 2016 at 02:49:59PM -0200, Ricardo Salveti wrote:
On Mon, Feb 8, 2016 at 2:37 PM, Steve McIntyre
Oh, ugh. What's the problem with the EFI variables? From a Debian perspective, you could (almost) get away with using the workaround I developed for broken firmware that doesn't do EFI properly. See http://bugs.debian.org/746662 for the full story if you're interested. There's support in Debian to track this automatically, and it's preseedable too if you need that.
Isn't the variables here the runtime support (for efibootmng)? If so, that is not supported by hikey.
:-(
However (as Mark says), if it's looking for (ESP)/EFI/BOOT/grubaa64.efi then it's wrong. The removable media path should be (ESP)/EFI/BOOT/bootaa64.efi, or have you mis-typed that in your mail here? (I hope so!)
This is probably why (which is known to be broken):
HisiPkg/HiKeyPkg/Drivers/HiKeyDxe/InstallBootMenu.c [HIKEY_BOOT_ENTRY_BOOT_EMMC] = { L"VenHw(B549F005-4BD4-4020-A0CB-06F42BDA68C3)/HD(6,GPT,5C0F213C-17E1-4149-88C8-8B50FB4EC70E,0x7000,0x20000)/\EFI\BOOT\GRUBAA64.EFI", NULL, L"boot from eMMC", LOAD_OPTION_CATEGORY_APP },
Oh, ick. I see. That's... special.
I'm hoping this to be fixed with the edk2/openplatformpkg rework and upstreaming effort that was started a few weeks ago.
I hope so.
Cheers,
On Mon, Feb 8, 2016 at 5:46 PM, Ricardo Salveti ricardo.salveti@linaro.org wrote:
On Mon, Feb 8, 2016 at 3:17 PM, Mark Rutland mark.rutland@arm.com wrote:
On Mon, Feb 08, 2016 at 02:49:59PM -0200, Ricardo Salveti wrote:
On Mon, Feb 8, 2016 at 2:37 PM, Steve McIntyre steve.mcintyre@linaro.org wrote:
On Mon, Feb 08, 2016 at 03:35:36PM +0000, Grant Likely wrote:
Hi all,
I've spent some time over the past couple of days getting update-grub working on the hikey. It works for me, but I need some review. The solution consists of three parts:
- a new config file in /etc/default/kernel that can contain
KERNEL_DEVICETREE="<board-name>" 2) a new script in /etc/kernel/postinst.d/devicetree that copies the requested device tree file into /boot when KERNEL_DEVICETREE is set 3) a modifcation to /etc/grub.d/10_linux that will add the devicetree command when KERNEL_DEVICETREE is set.
Hmmmm. Other machines aren't needing to know the specific devicetree in this way.
That is probably because the firmware is providing a valid device tree by default.
I've also be partially successful using the grub-install tool. It will install the grubaa64.efi application correctly in (ESP)/EFI/debian/grubaa64.efi, but barfs when trying to update the EFI variables which is unsurprising. However, the system boots fine when I copy the installed grubaa64.efi into (ESP)/EFI/BOOT/grubaa64.efi
Oh, ugh. What's the problem with the EFI variables? From a Debian perspective, you could (almost) get away with using the workaround I developed for broken firmware that doesn't do EFI properly. See http://bugs.debian.org/746662 for the full story if you're interested. There's support in Debian to track this automatically, and it's preseedable too if you need that.
Isn't the variables here the runtime support (for efibootmng)?
Yes. The boot variables are like any other variables, and are manipulated via runtime services (e.g. SetVariable).
If so, that is not supported by hikey.
Is there any route to having them supported? I don't know enough about HiKey to know whether it's not possible, very difficult, or simply not worked on.
They are required per the EFI spec, and it's an unfortunate divergence given they're key to the usual boot flow.
Haojian, since you are restructuring the code as we speak, would it be even possible to support runtime here?
Since you are already storing meta-data in a specific disk partition, wouldn't it be possible to reuse the same logic to make runtime supported?
The hard part of SetVariable on eMMC is that he firmware cannot touch the eMMC device while the kernel is running. One of two things must be done: 1) Store all changes in memory and wait until ResetSystem() is called to write out to eMMC. Downside: Changes are lost if the system crashes/reboots without calling ResetSystem() 2) Have a kernel driver that stores variable updates on UEFI's behalf. UEFI will need to call back out to the kernel to request write cycles Downside: violates UEFI spec not requiring any kernel involvement to store variables --- However, given the nature of the hardware, I don't think this violates the intent of the spec and I believe there is work in progress to get the spec adjusted for the eMMC use case.
g.
On Tue, Feb 9, 2016 at 1:11 PM, Steve McIntyre steve.mcintyre@linaro.org wrote:
On Mon, Feb 08, 2016 at 02:49:59PM -0200, Ricardo Salveti wrote:
On Mon, Feb 8, 2016 at 2:37 PM, Steve McIntyre
Oh, ugh. What's the problem with the EFI variables? From a Debian perspective, you could (almost) get away with using the workaround I developed for broken firmware that doesn't do EFI properly. See http://bugs.debian.org/746662 for the full story if you're interested. There's support in Debian to track this automatically, and it's preseedable too if you need that.
Isn't the variables here the runtime support (for efibootmng)? If so, that is not supported by hikey.
:-(
However (as Mark says), if it's looking for (ESP)/EFI/BOOT/grubaa64.efi then it's wrong. The removable media path should be (ESP)/EFI/BOOT/bootaa64.efi, or have you mis-typed that in your mail here? (I hope so!)
This is probably why (which is known to be broken):
HisiPkg/HiKeyPkg/Drivers/HiKeyDxe/InstallBootMenu.c [HIKEY_BOOT_ENTRY_BOOT_EMMC] = { L"VenHw(B549F005-4BD4-4020-A0CB-06F42BDA68C3)/HD(6,GPT,5C0F213C-17E1-4149-88C8-8B50FB4EC70E,0x7000,0x20000)/\EFI\BOOT\GRUBAA64.EFI", NULL, L"boot from eMMC", LOAD_OPTION_CATEGORY_APP },
Oh, ick. I see. That's... special.
Yup. We're blazing new trails here. Aside from enterprise class systems*, all ARM UEFI ports are dealing with this pain. However, once we've got it sorted for HiKey then that solution should pretty much carry over to all platforms with eMMC and UEFI.
g.
* Enterprise class systems generally have completely separate storage for firmware that look remarkably like equivalent x86 hardware.
On Mon, Feb 8, 2016 at 4:37 PM, Steve McIntyre steve.mcintyre@linaro.org wrote:
On Mon, Feb 08, 2016 at 03:35:36PM +0000, Grant Likely wrote:
Hi all,
I've spent some time over the past couple of days getting update-grub working on the hikey. It works for me, but I need some review. The solution consists of three parts:
- a new config file in /etc/default/kernel that can contain
KERNEL_DEVICETREE="<board-name>" 2) a new script in /etc/kernel/postinst.d/devicetree that copies the requested device tree file into /boot when KERNEL_DEVICETREE is set 3) a modifcation to /etc/grub.d/10_linux that will add the devicetree command when KERNEL_DEVICETREE is set.
Hmmmm. Other machines aren't needing to know the specific devicetree in this way.
As already discussed, this is an issue that firmware doesn't yet supply a DT. We will get this fixed on HiKey, but that isn't the only reason for doing this. During development it will still be important to override the firmware provided DT. There needs to be some mechanism for making update-grub respect that.
I'm happy to change the rules by which the loaded devicetree is chosen. It doesn't have to require the DTB to be shipped in the kernel package (I merely chose that because it was convenient), and it doesn't need to be based on the kernel version number (although that may also prove to be useful at times). Just as long as the functionality is there to get update-grub to insert the devicetree line when the user requires it.
g.
-
Grant Likely -
Mark Rutland -
Ricardo Salveti -
Steve McIntyre