Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for running macOS guests. #148

Merged
merged 3 commits into from
Aug 28, 2024

Conversation

williamtheaker
Copy link
Contributor

@williamtheaker williamtheaker commented May 28, 2024

This PR implements running macOS guests with vfkit, which requires an arm64/Apple silicon device running macOS 12 or later. Thank you cfergeau for your assistance in developing this feature.

Core concepts

The macOS in QEMU (ARM edition) talk explains this in more detail, but booting a macOS guest image requires three components:

  • An auxiliary storage file with NVRAM contents and the iBoot bootloader
  • hardwareModel - A binary property list defining OS version support
  • MachineIdentifier - unique identifier for the VM

These files can be created with Apple's sample InstallationTool code or by building and running the installation example in the vz repo.

Tested working

Host running 14.5 (23F79) and guest running 14.4.1 (23E224):

  • Networking
  • Input. I didn't implement VZMacKeyboardConfiguration (macOS 14+) or VZMacTrackpadConfiguration (macOS 13+) since the basic virtio input works fine and I don't need gestures.
  • SSH. Enabling it in the guest via System Preferences and grab the VM ip from the host /var/db/dhcpd_leases.
  • Adding a virtio-serial device creates a file at /dev/tty.virtio.
  • RESTful API. It seems like GUI does not close with state HardStop #67 impacts macOS guests too.
  • Sharing directories from host to guest

Unimplemented

  • Downloading .ipsw restore images and creating new base install images
  • Audio, since vfkit hasn't implemented it yet.
  • Various features requiring macOS version 14+:
  • Testing macOS vms similar to the existing puipui-linux tests

Todo

Figure out how to handle metadata. Other projects like macosvm and Tart create .json files with the VM's hardwareModel and MachineIdentifier values. Maybe this would be a good use for #28 too?

Closes #139

Copy link

openshift-ci bot commented May 28, 2024

Hi @williamtheaker. Thanks for your PR.

I'm waiting for a crc-org member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@cfergeau
Copy link
Collaborator

cfergeau commented Jun 5, 2024

/ok-to-test

@cfergeau
Copy link
Collaborator

cfergeau commented Jun 5, 2024

Handle x86 so the Makefile doesn't need to be partially commented out.

I've fixed this in https://github.com/cfergeau/vfkit/tree/wt.macos_guests
Only tried to compile, did not test runtime behaviour

@cfergeau
Copy link
Collaborator

cfergeau commented Jun 5, 2024

Do you have an example of the json file used by macosvm or tart?

  • Fix hardcoded values AuxiliaryStorageVar, HardwareModelVar, and MachineIdentifierVar in pkg/vf/vm.go. I could use your help @cfergeau figuring out how to pass the macOS bootloader arguments to NewMacPlatformConfiguration().

I'll try to look at this today!

@williamtheaker
Copy link
Contributor Author

Do you have an example of the json file used by macosvm or tart?

macosvm

{
  "serial": true,
  "os": "macos",
  "hardwareModel": "YnBsaXN0MDDTAQIDBAUGXxAZRGF0YVJlcHJlc2VudGF0aW9uVmVyc2lvbl8QD1BsYXRmb3JtVmVyc2lvbl8QEk1pbmltdW1TdXBwb3J0ZWRPUxQAAAAAAAAAAAAAAAAAAAABEAKjBwgIEA0QAAgPKz1SY2VpawAAAAAAAAEBAAAAAAAAAAkAAAAAAAAAAAAAAAAAAABt",
  "storage": [
    {
      "type": "disk",
      "file": "disk.img"
    },
    {
      "type": "aux",
      "file": "aux.img"
    }
  ],
  "ram": 4294967296,
  "machineId": "YnBsaXN0MDDRAQJURUNJRBMlAGfNcIpmowgLEAAAAAAAAAEBAAAAAAAAAAMAAAAAAAAAAAAAAAAAAAAZ",
  "displays": [
    {
      "dpi": 200,
      "height": 1600,
      "width": 2560
    }
  ],
  "version": 1,
  "cpus": 2,
  "networks": [
    {
      "type": "nat"
    }
  ],
  "audio": false
}

Tart

{
  "macAddress" : "c2:0e:15:c5:6e:25",
  "memorySizeMin" : 4294967296,
  "display" : {
    "width" : 1024,
    "height" : 768
  },
  "hardwareModel" : "YnBsaXN0MDDTAQIDBAUGXxAZRGF0YVJlcHJlc2VudGF0aW9uVmVyc2lvbl8QD1BsYXRmb3JtVmVyc2lvbl8QEk1pbmltdW1TdXBwb3J0ZWRPUxQAAAAAAAAAAAAAAAAAAAABEAKjBwgIEA0QAAgPKz1SY2VpawAAAAAAAAEBAAAAAAAAAAkAAAAAAAAAAAAAAAAAAABt",
  "cpuCount" : 4,
  "os" : "darwin",
  "ecid" : "YnBsaXN0MDDRAQJURUNJRBQAAAAAAAAAAMSGIoemH\/\/VCAsQAAAAAAAAAQEAAAAAAAAAAwAAAAAAAAAAAAAAAAAAACE=",
  "version" : 1,
  "memorySize" : 4294967296,
  "arch" : "arm64",
  "cpuCountMin" : 2
}

You can see the hardwareModel is identical in both files. It's a binary plist containing {"DataRepresentationVersion":1,"MinimumSupportedOS":[13,0,0],"PlatformVersion":2}. I need to see if I can use that with vz.NewMacMachineIdentifierWithData as a fallback.

I'm still trying to figure out whether it makes sense to add the ability to create/install macOS VMs with vfkit, which will influence how to provide these values, whether as strings or files on disk.

@williamtheaker
Copy link
Contributor Author

Handle x86 so the Makefile doesn't need to be partially commented out.

I've fixed this in https://github.com/cfergeau/vfkit/tree/wt.macos_guests Only tried to compile, did not test runtime behaviour

Thanks! Should I merge your architecture-specific files commit into my branch?

@cfergeau
Copy link
Collaborator

cfergeau commented Jun 5, 2024

Handle x86 so the Makefile doesn't need to be partially commented out.

I've fixed this in https://github.com/cfergeau/vfkit/tree/wt.macos_guests Only tried to compile, did not test runtime behaviour

Thanks! Should I merge your architecture-specific files commit into my branch?

Sure, you can do whatever you see fit with them, cherry-pick, squash, rewrite, ... :)

@cfergeau
Copy link
Collaborator

cfergeau commented Jun 5, 2024

Fix hardcoded values AuxiliaryStorageVar, HardwareModelVar, and MachineIdentifierVar in pkg/vf/vm.go. I could use your help @cfergeau figuring out how to pass the macOS bootloader arguments to NewMacPlatformConfiguration().

I added a few more commits to https://github.com/cfergeau/vfkit/tree/wt.macos_guests to pass the args to NewMacPlatformConfiguration.
There seems to be a bigger question though, where should these values be coming from, who creates them, ..., do they need to be stored in a directory in ~/Library/Application\ Support/vfkit, ...
I need to read more about macos VMs to be able to give meaningful advice about this.

I'm still trying to figure out whether it makes sense to add the ability to create/install macOS VMs with vfkit, which will influence how to provide these values, whether as strings or files on disk.

Same answer as in the previous paragraph, I'll need to look closer at this. My gut feeling is that it would make sense to offer this, it's (a bit) similar to #124 / #123

@cfergeau
Copy link
Collaborator

cfergeau commented Jun 6, 2024

If the code is this PR is enough to be able to start and use an already installed macos VM, I think we should work on merging this first. The other missing features can be added on top of this in separate PRs, no need to block the merging of this until everything is done.

@williamtheaker williamtheaker marked this pull request as ready for review June 17, 2024 16:19
@openshift-ci openshift-ci bot requested review from cfergeau and praveenkumar June 17, 2024 16:19
@williamtheaker
Copy link
Contributor Author

I merged in your commits and fixed some linting issues. I think this is ready to be reviewed.

@williamtheaker
Copy link
Contributor Author

There's another tool using the Virtualization Framework called Orka, which, like Tart, stores images and "layers" (not sure how closely they match Linux container functionality) in an OCI format. The software is proprietary and it uses nonstandard mediatypes, but we can still look at their config file layout. Here's the config from this Orka base image:

{
  "UID": "882B4402-E965-43E4-9570-EA01EEDCE5AA",
  "cpuCount": 4,
  "diskSizeGB": 90,
  "displayDPI": 96,
  "displayHeight": 1080,
  "displayWidth": 1920,
  "graphicalConsole": false,
  "hardwareModel": "YnBsaXN0MDDTAQIDBAUGXxAZRGF0YVJlcHJlc2VudGF0aW9uVmVyc2lvbl8QD1BsYXRmb3JtVmVyc2lvbl8QEk1pbmltdW1TdXBwb3J0ZWRPUxQAAAAAAAAAAAAAAAAAAAABEAKjBwgIEA0QAAgPKz1SY2VpawAAAAAAAAEBAAAAAAAAAAkAAAAAAAAAAAAAAAAAAABt",
  "installed": true,
  "machineIdentifier": "YnBsaXN0MDDRAQJURUNJRBMT1iLyrVR8AggLEAAAAAAAAAEBAAAAAAAAAAMAAAAAAAAAAAAAAAAAAAAZ",
  "memorySize": 4294967296,
  "memorySizeMB": 4096,
  "restoreImage": "default",
  "serialConsole": true,
  "version": "8C89B5C1-3D6E-4B80-A45B-51EE97B6C0EB"
}

Looks like everyone is using the same generic hardwareModel string 😅

@cfergeau
Copy link
Collaborator

This hardware model string is not really special/magic:

$ echo YnBsaXN0MDDTAQIDBAUGXxAZRGF0YVJlcHJlc2VudGF0aW9uVmVyc2lvbl8QD1BsYXRmb3JtVmVyc2lvbl8QEk1pbmltdW1TdXBwb3J0ZWRPUxQAAAAAAAAAAAAAAAAAAAABEAKjBwgIEA0QAAgPKz1SY2VpawAAAAAAAAEBAAAAAAAAAAkAAAAAAAAAAAAAAAAAAABt |base64 -d |plistutil -i -

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
	<key>DataRepresentationVersion</key>
	<integer>1</integer>
	<key>PlatformVersion</key>
	<integer>2</integer>
	<key>MinimumSupportedOS</key>
	<array>
		<integer>13</integer>
		<integer>0</integer>
		<integer>0</integer>
	</array>
</dict>
</plist>

However, I guess Apple does not document it, so better to treat it as an opaque blob.

@williamtheaker
Copy link
Contributor Author

There's a comment in pkg/vf/vm_arm64.go with the plist payload. Since the value seems to be unchanged since 2022, maybe in a future commit we can default to using it as a hardcoded value when no hardwareModel argument is supplied.

Copy link
Collaborator

@cfergeau cfergeau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Finally got back to this PR, I squashed/split the commits a bit, and dropped the comments which were asking questions about potential fixes.
I've force-pushed these changes to this PR, I'm fine to merge these as they are.

README.md Outdated
@@ -17,6 +17,9 @@ brew tap cfergeau/crc
brew install vfkit
```

### Building

From the root direction of this repository, run `make`. You will need a valid Apple Developer certificate to sign the binaries with the required entitlements.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A valid Apple Developer certificate is not required to sign the binary, the Makefile uses codesign -s - aka ad hoc signing ( https://ss64.com/mac/codesign.html ), which works without any Apple certificate.

@@ -5,14 +5,14 @@
vfkit is a macOS command-line-based hypervisor, which uses [Apple's Virtualization Framework](https://developer.apple.com/documentation/virtualization?language=objc) to run virtual machines.
You start a virtual machine by running vfkit with a set of arguments describing the virtual machine configuration/hardware.
When vfkit stops, the virtual machine stops running.
It requires macOS 11 or newer, and runs on both x86_64 and aarch64 Macs.
It requires macOS 11 or newer, and runs on both x86_64 and arm64 Macs.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I usually use either amd64/arm64 which is the go naming, or x86_64/aarch64 which is the linux naming, x86_64+arm64 is imo weirdly inconsistent. We can use Intel and Apple silicon Macs which is the naming used by Apple.

```HTTP
GET /vm/inspect
```

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for all the typo/formatting/... fixes, I've split them to their own commit.

pkg/vf/virtio.go Outdated
@@ -328,6 +340,7 @@ func (dev *USBMassStorage) AddToVirtualMachineConfig(vmConfig *VirtualMachineCon
return nil
}

// Move these to top of file?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed this comment, but yeah it would make sense to group all hte public structs at the top of the file.

)

func NewMacPlatformConfiguration(machineIdentifierVar, hardwareModelVar, auxiliaryStorageVar string) (vz.PlatformConfiguration, error) {
// var HardwareModelVar =[]byte( "YnBsaXN0MDDTAQIDBAUGXxAZRGF0YVJlcHJlc2VudGF0aW9uVmVyc2lvbl8QD1BsYXRmb3JtVmVyc2lvbl8QEk1pbmltdW1TdXBwb3J0ZWRPUxQAAAAAAAAAAAAAAAAAAAABEAKjBwgIEA0QAAgPKz1SY2VpawAAAAAAAAEBAAAAAAAAAAkAAAAAAAAAAAAAAAAAAABt") // Binary plist with {"DataRepresentationVersion":1,"MinimumSupportedOS":[13,0,0],"PlatformVersion":2}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've kept the comment, but explained a bit more what this is about:

        // The following string is common for the hardware model:
        // `YnBsaXN0MDDTAQIDBAUGXxAZRGF0YVJlcHJlc2VudGF0aW9uVmVyc2lvbl8QD1BsYXRmb3JtVmVyc2lvbl8QEk1pbmltdW1TdXBwb3J0ZWRPUxQAAAAAAAAAAAAAAAAAAAABEAKjBwgIEA0QAAgPKz1SY2VpawAAAAAAAAEBAAAAAAAAAAkAAAAAAAAAAAAAAAAAAABt`
        // It is a base64-encoded binary plist with this content: `{"DataRepresentationVersion":1,"MinimumSupportedOS":[13,0,0],"PlatformVersion":2}`

@@ -172,7 +172,8 @@ func runVirtualMachine(vmConfig *config.VirtualMachine, vm *vf.VirtualMachine) e
log.Debugf("%v", err)
}

log.Infof("waiting for VM to stop")
// Is this print statement necessary? It gives the impression that any running VM is in the process of stopping
log.Infof("Waiting for VM to stop")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've dropped these changes for now, but I agree this could be improved.

Copy link

openshift-ci bot commented Aug 23, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cfergeau

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@cfergeau cfergeau merged commit 4b951b2 into crc-org:main Aug 28, 2024
3 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Running macOS guests
2 participants