New vendor branch for FreeBSD on Hyper-V. Please review.

Alexander Motin mav at
Wed Mar 27 07:17:09 UTC 2013


As ATA driver maintainer I've took a look on that part of the code. I 
have some comments about what I see. Please keep me CC'd, as I am 
normally not on this list.

1) You are using machine dependent function do_cpuid() in machine 
independent ATA code. It will just not build on any architecture except 
amd64 and i386. You should periodically run `make universe` to be sure 
that you code does not breaks the build for some rare archs. As minimum 
that code should be wrapped into respective #ifdef's, but I would prefer 
it to be moved out somewhere to MD code (like MD code that does general 
VM detection now), as this doesn't really belong to the ATA driver.

2) You are disabling "ad" ATA disk driver. Here I have two objections:
  a) "ad" driver is obsoleted and not used since FreeBSD 9.0, when 
CAM-based ATA stack replaces it completely, wrapping only controller 
driver parts of old ata(4) into CAM SIM, which is now only one of four 
ATA/SATA SIMs in the system. Your change is effectively null now.
  b) I believe you are using wrong approach here. I believe instead of 
completely disabling disk driver you should disable only specific 
Hyper-V emulated ATA controller device instance. I've never used 
Hyper-V, but I guess it should support PCI device pass-through. Just 
imagine what happen if somebody wish to pass-through ATA/SATA 
controller? You will disable it also.

3) Looking on what I suppose is "Fast IDE" driver in the file 
hv_storvsc_drv_freebsd.c, it seems more like cut-down SAS, then IDE. It 
doesn't support ATA commands, it reports itself to CAM as SAS. The only 
differences I see are disabling LUNs and limiting queue to 1 slot. Just 
a smile: it is neither really "Fast" nor at all "IDE". :)

4) In you SAS driver in storvsc_action() you have hardcoded 
CTS_SCSI_FLAGS_TAG_ENB flag. While I guess it should not cause any 
problems, it wont allow user to control it if he decide to experiment 
with disabling it.

Alexander Motin

More information about the freebsd-virtualization mailing list