Hi Gautam, Thanks for reviewing this patch. My responses to your review comments inline below: Gautam Menghani <gautam@xxxxxxxxxxxxx> writes: > On Sun, Dec 22, 2024 at 07:32:29PM +0530, Vaibhav Jain wrote: >> Update kvm-nested APIv2 documentation to include five new >> Guest-State-Elements to fetch the hostwide counters. These counters are >> per L1-Lpar and indicate the amount of Heap/Page-table memory allocated, >> available and Page-table memory reclaimed for all L2-Guests active >> instances >> >> Signed-off-by: Vaibhav Jain <vaibhav@xxxxxxxxxxxxx> >> --- >> Documentation/arch/powerpc/kvm-nested.rst | 40 ++++++++++++++++------- >> 1 file changed, 29 insertions(+), 11 deletions(-) >> >> diff --git a/Documentation/arch/powerpc/kvm-nested.rst b/Documentation/arch/powerpc/kvm-nested.rst >> index 5defd13cc6c1..c506192f3f98 100644 >> --- a/Documentation/arch/powerpc/kvm-nested.rst >> +++ b/Documentation/arch/powerpc/kvm-nested.rst >> @@ -208,13 +208,9 @@ associated values for each ID in the GSB:: >> flags: >> Bit 0: getGuestWideState: Request state of the Guest instead >> of an individual VCPU. >> - Bit 1: takeOwnershipOfVcpuState Indicate the L1 is taking >> - over ownership of the VCPU state and that the L0 can free >> - the storage holding the state. The VCPU state will need to >> - be returned to the Hypervisor via H_GUEST_SET_STATE prior >> - to H_GUEST_RUN_VCPU being called for this VCPU. The data >> - returned in the dataBuffer is in a Hypervisor internal >> - format. >> + Bit 1: getHostWideState: Request stats of the Host. This causes >> + the guestId and vcpuId parameters to be ignored and attempting >> + to get the VCPU/Guest state will cause an error. > > s/Request stats/Request state > Its is 'Request stats' are this flag currently return Hostwide stat counters. >> Bits 2-63: Reserved >> guestId: ID obtained from H_GUEST_CREATE >> vcpuId: ID of the vCPU pass to H_GUEST_CREATE_VCPU >> @@ -402,13 +398,14 @@ GSB element: >> >> The ID in the GSB element specifies what is to be set. This includes >> archtected state like GPRs, VSRs, SPRs, plus also some meta data about >> -the partition like the timebase offset and partition scoped page >> +the partition and like the timebase offset and partition scoped page >> table information. > > The statement that is already there looks correct IMO. > Right. I will update it to reflect L1-lpar stats in v2. >> >> +--------+-------+----+--------+----------------------------------+ >> -| ID | Size | RW | Thread | Details | >> -| | Bytes | | Guest | | >> -| | | | Scope | | >> +| ID | Size | RW |(H)ost | Details | >> +| | Bytes | |(G)uest | | >> +| | | |(T)hread| | >> +| | | |Scope | | >> +========+=======+====+========+==================================+ >> | 0x0000 | | RW | TG | NOP element | >> +--------+-------+----+--------+----------------------------------+ >> @@ -434,6 +431,27 @@ table information. >> | | | | |- 0x8 Table size. | >> +--------+-------+----+--------+----------------------------------+ >> | 0x0007-| | | | Reserved | >> +| 0x07FF | | | | | >> ++--------+-------+----+--------+----------------------------------+ >> +| 0x0800 | 0x08 | R | H | Current usage in bytes of the | >> +| | | | | L0's Guest Management Space | >> ++--------+-------+----+--------+----------------------------------+ >> +| 0x0801 | 0x08 | R | H | Max bytes available in the | >> +| | | | | L0's Guest Management Space | >> ++--------+-------+----+--------+----------------------------------+ >> +| 0x0802 | 0x08 | R | H | Current usage in bytes of the | >> +| | | | | L0's Guest Page Table Management | >> +| | | | | Space | >> ++--------+-------+----+--------+----------------------------------+ >> +| 0x0803 | 0x08 | R | H | Max bytes available in the L0's | >> +| | | | | Guest Page Table Management | >> +| | | | | Space | >> ++--------+-------+----+--------+----------------------------------+ >> +| 0x0804 | 0x08 | R | H | Amount of reclaimed L0 Guest's | >> +| | | | | Page Table Management Space due | >> +| | | | | to overcommit | > > I think it would be more clear to specify "... Management space for L1 > ..." in the details of all above entries. > Agree, Updated that in v2. >> ++--------+-------+----+--------+----------------------------------+ >> +| 0x0805-| | | | Reserved | >> | 0x0BFF | | | | | >> +--------+-------+----+--------+----------------------------------+ >> | 0x0C00 | 0x10 | RW | T |Run vCPU Input Buffer: | >> -- > > Also, the row 2 of this table mentions the 'takeOwnershipOfVcpuState' flag > which is no longer supported. You can remove that as well. > Updating this document for 'takeOwnershipOfVcpuState' will be done as a separate patch and not part of this patch series >> 2.47.1 >> > -- Cheers ~ Vaibhav
