From: Marc Orr <firstname.lastname@example.org> To: Sean Christopherson <email@example.com> Cc: firstname.lastname@example.org, Jim Mattson <email@example.com>, Peter Shier <firstname.lastname@example.org> Subject: Re: [kvm-unit-tests PATCH] x86: nvmx: test max atomic switch MSRs Date: Fri, 13 Sep 2019 14:55:56 -0700 Message-ID: <CAA03e5E28QaDAHjCg5J0_aPoY8pNnUiUQVvrZSHsEj0dq6email@example.com> (raw) In-Reply-To: <20190913183040.GA8904@linux.intel.com> Thanks for the review! I'll get to work on v2 now. > > I'll happily apply what you've suggested in v2. But I don't see why > > it's so terrible to over-allocate here. Leveraging a generic 2 MB page > > allocator can be reused going forward, and encourages uniformity > > across tests. > > My main concern is avoiding setting 6mb+ of memory. I like to run the > tests in L1 and L2 and would prefer to keep overhead at a minimum. > > As for the allocation itself, being precise in the allocation size is a > form of documentation, e.g. it conveys that the size/order was chosen to > ensure enough space for the maximum theoretical list size. A purely > arbitrary size, especially one that corresponds with a large page size, > can lead to people looking for things that don't exist, e.g. the 2mb size > is partially what led me to believe that this test was deliberately > exceeding the limit, otherwise why allocate such a large amount of memory? > I also didn't know if 2mb was sufficient to handle the maximum theoretical > list size. SGTM. I'll make this change in v2. > > > Distilling things down to the bare minimum yields something like the > > > following. > > > > Looks excellent overall. Still not clear what the consensus is on > > whether or not to test the VM-entry failure. I think a flag seems like > > a reasonable compromise. I've never added a flag to a kvm-unit-test, > > so I'll see if I can figure that out. > > No need for a flag if you want to go that route, just put it in a separate > VMX subtest and exclude said test from the [vmx] config, i.e. make the > test opt-in. SGTM, thanks!
next prev parent reply index Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-09-12 18:09 Marc Orr 2019-09-13 15:24 ` Sean Christopherson 2019-09-13 16:26 ` Jim Mattson 2019-09-13 17:15 ` Sean Christopherson 2019-09-13 17:21 ` Jim Mattson 2019-09-13 18:02 ` Marc Orr 2019-09-13 18:30 ` Sean Christopherson 2019-09-13 21:55 ` Marc Orr [this message] 2019-09-14 0:49 ` Marc Orr
Reply instructions: You may reply publically to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CAA03e5E28QaDAHjCg5J0_aPoY8pNnUiUQVvrZSHsEj0dq6firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
KVM Archive on lore.kernel.org Archives are clonable: git clone --mirror https://lore.kernel.org/kvm/0 kvm/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 kvm kvm/ https://lore.kernel.org/kvm \ firstname.lastname@example.org email@example.com public-inbox-index kvm Newsgroup available over NNTP: nntp://nntp.lore.kernel.org/org.kernel.vger.kvm AGPL code for this site: git clone https://public-inbox.org/ public-inbox