hyperv/storvsc: disable ata but enable CD/DVD and remove stordisengage

[honzhan_microsoft.com]

Summary

If we changes the IDE lun for CD/DVD and OS disk, sometimes the boot
will fail because of failing to attach the disk device. There are two reasons:

1. stordisengage driver does not work as we expected (disable the loading of ATA driver).

It failed to stop the loading of ATA drivers.

2. if the storvsc find an invalid IDE lun, reporting CAM_SEL_TIMEOUT to

CAM layer is correct.

This fix disabled ATA driver if FreeBSD is started on Hyper-V VM.

Submitted by: Hongjiang Zhang <honzhan microsoft com>

[mav]

From the beginning I was against hacking CAM to workaround Hyper-V problems. Here it returns again. What is exactly wrong with the hv_ata_pci_disengage.c approach?

[honzhan_microsoft.com]

In D7693#159708, @mav wrote:

From the beginning I was against hacking CAM to workaround Hyper-V problems. Here it returns again. What is exactly wrong with the hv_ata_pci_disengage.c approach?

Hyper-V manager provides 2 IDE controllers, and each controller has 2 luns. The default locations for CD/DVD and OS disk work fine. But if you change the IDE controller of CD/DVD and OS disk, or merge them to locate in the same IDE controller. There will be something wrong happened: sometimes CD/DVD device disappears, which will cause provisioning failure for FreeBSD on Azure.

[honzhan_microsoft.com]

I did not find a workable solution except to hack CAM until now. If you have suggestions, I'd like to try. The hv_ata_pci_disengage.c approach cannot determined which IDE controller CD/DVD locates in "probe" method.

[mav]

In D7693#159711, @honzhan_microsoft.com wrote:

Hyper-V manager provides 2 IDE controllers, and each controller has 2 luns. The default locations for CD/DVD and OS disk work fine. But if you change the IDE controller of CD/DVD and OS disk, or merge them to locate in the same IDE controller.

OK, so one thing that I was told never happen did happen.

Then let me remind you one more of my arguments that I was told to never happen: your patch completely disables ATA disks, not looking where they are connected, so it can easily be real ATA disk, connected to passed through PCIe SATA controller, that as I understand is going to be supported in 2016 version.

[honzhan_microsoft.com]

Then let me remind you one more of my arguments that I was told to never happen: your patch completely disables ATA disks, not looking where they are connected, so it can easily be real ATA disk, connected to passed through PCIe SATA controller, that as I understand is going to be supported in 2016 version.

Yes, I agree this patch brutally disabled ATA without checking anything. IMHO if ATA driver can be inherited by the customized ATA driver, for example, hv_ata_pci_disengage.c, which sets a flag or invokes a callback to inform the default ATA driver that it should be disabled. Unfortunately, I did not find any mechanism to stop ATA with a decent way.

[honzhan_microsoft.com]

To understand the issue, I captured the DVD and OS Disk configuration on Hyper-V manager. From the picture, we can see there are 2 IDE Controllers and each IDE Controller has 2 locations. Changing the controller for DVD and OS Disk will cause DVD missing on VM side.

[sepherosa_gmail.com]

In D7693#159716, @mav wrote:

In D7693#159711, @honzhan_microsoft.com wrote:

Hyper-V manager provides 2 IDE controllers, and each controller has 2 luns. The default locations for CD/DVD and OS disk work fine. But if you change the IDE controller of CD/DVD and OS disk, or merge them to locate in the same IDE controller.

OK, so one thing that I was told never happen did happen.

Then let me remind you one more of my arguments that I was told to never happen: your patch completely disables ATA disks, not looking where they are connected, so it can easily be real ATA disk, connected to passed through PCIe SATA controller, that as I understand is going to be supported in 2016 version.

Pass-through support is a good argument. Hmm, how about this? We move further on the probe path to PROBE_IDENTIFY, so we can have the disk model to filter, the virtual disk is <Msft blah blah>, we just ignore those ones.

[sepherosa_gmail.com]

sys/dev/hyperv/storvsc/hv_storvsc_drv_freebsd.c

2207 ↗

(On Diff #19824)

This lock is actually not useful for xpt_print()

[mav]

In D7693#159890, @sepherosa_gmail.com wrote:

Pass-through support is a good argument. Hmm, how about this? We move further on the probe path to PROBE_IDENTIFY, so we can have the disk model to filter, the virtual disk is <Msft blah blah>, we just ignore those ones.

This could probably look better. There are concepts of model-based quirks on both transport and peripheral layers of CAM. There could probably be added new quirk to either completely ignore device, aborting its probe on transport layer, or only block ada driver from attachment, if there is any reason to keep ATA commands pass-through to it.

[mav]

One more downside of this CAM modification approach is that if previously user not loading hyperv drivers automatically got legacy emulation ATA, after this change -- he will get no disks at all.

[sepherosa_gmail.com]

In D7693#160046, @mav wrote:

In D7693#159890, @sepherosa_gmail.com wrote:

Pass-through support is a good argument. Hmm, how about this? We move further on the probe path to PROBE_IDENTIFY, so we can have the disk model to filter, the virtual disk is <Msft blah blah>, we just ignore those ones.

This could probably look better. There are concepts of model-based quirks on both transport and peripheral layers of CAM. There could probably be added new quirk to either completely ignore device, aborting its probe on transport layer, or only block ada driver from attachment, if there is any reason to keep ATA commands pass-through to it.

Well, we changed the plan a little bit. Filtering the model is not going to work here, since you can pass-through a physical disk, if that happens the model string will be the physical disk's model string. So what Hongjiang did in the new patch is to abort probing the ATA disk hooked under the emulated ATA controller (Intel PIIX4, I don't believe it will be used for PCI passthrough ever).

[sepherosa_gmail.com]

In D7693#160050, @mav wrote:

One more downside of this CAM modification approach is that if previously user not loading hyperv drivers automatically got legacy emulation ATA, after this change -- he will get no disks at all.

OK, reasonable concern. It can be solved by setting a flag in vm_guest to indicate that Hyper-V SCSI driver will take over the disks.

[sepherosa_gmail.com]

In D7693#160050, @mav wrote:

One more downside of this CAM modification approach is that if previously user not loading hyperv drivers automatically got legacy emulation ATA, after this change -- he will get no disks at all.

How about the latest patch? The changes to the CAM (5 lines of effective code) is much less invasive than previous ones, and I think it address various concerns properly.

[sepherosa_gmail.com]

In D7693#163011, @sepherosa_gmail.com wrote:

In D7693#160050, @mav wrote:

One more downside of this CAM modification approach is that if previously user not loading hyperv drivers automatically got legacy emulation ATA, after this change -- he will get no disks at all.

How about the latest patch? The changes to the CAM (5 lines of effective code) is much less invasive than previous ones, and I think it address various concerns properly.

If no objection comes, these will be committed tomorrow.